patterncppMajor
Simple string reverse program in C++
Viewed 0 times
programsimplestringreverse
Problem
I want to learn how to write good code. Starting off with simple programs for now. So any suggestions regarding style, readability or more efficient ways of implementation, etc. would really be appreciated!
#include
#include
using namespace std;
int main(int argc, char** argv)
{
bool running = true;
while(running == true)
{
string input;
cout << "[Enter 'exit' to exit the program]\n";
cout << "Enter the text you want to reverse: ";
getline(cin, input);
if (input.compare("exit") == 0)
{
running = false;
break;
}
reverse(input.begin(), input.end());
cout << "The reversed version of your text is: " << input << endl << endl;
}
return 0;
}Solution
Don't
It is considered bad practice because of possible name collisions, ... Although nobody will hurt you, as this is such a small program :)
Don't compare booleans to booleans
Things like
You don't need
The variable
Why are you using
Seeing
is more clear.
Don't flush 2 times in a row (or at all)
Note that on some platforms (or if you need to), you have to flush the steam to see the output. In this case it is better to be explicit and use
You're not using the command line parameters
So why name them? You can just omit the names, or define a
I prefer option 2.
Your code uses implementation specific behavior
Technically, you don't need
Only for
using namespace std;It is considered bad practice because of possible name collisions, ... Although nobody will hurt you, as this is such a small program :)
Don't compare booleans to booleans
Things like
running == true are completely unnecessary, as running is already a condition in itself. Just use running.You don't need
runningThe variable
running is unnecessary. You need one for nested loops (breaking out of both), but for single loops, a single break and an infinite loop is preferred.Why are you using
compare?Seeing
input.compare("exit") == 0 would send me right to the docs, as I don't know what the return value of compare is.if (input == "exit");is more clear.
Don't flush 2 times in a row (or at all)
std::endl prints a new line, and then flushes stdout. This results in a performance hit, so it is better to output an actual newline. In your case:std::cout << something << "\n\n";Note that on some platforms (or if you need to), you have to flush the steam to see the output. In this case it is better to be explicit and use
std::flush.You're not using the command line parameters
So why name them? You can just omit the names, or define a
main which takes no parameters:int main(int, char**) {} //1)
int main() {} //2)I prefer option 2.
Your code uses implementation specific behavior
std::string is defined in the string header. Not every compiler includes string with iostream or algorithm (VS doesn't), so you have to include it explicitly. Don't rely on automatic includes.Technically, you don't need
return 0;Only for
main, if you omit return 0;, the compiler will add it for you, so technically, you don't need to specify it.Code Snippets
if (input == "exit");std::cout << something << "\n\n";int main(int, char**) {} //1)
int main() {} //2)Context
StackExchange Code Review Q#142201, answer score: 27
Revisions (0)
No revisions yet.