Good Code vs Bad Code

When writing code in any language, there are good coding practices and there are really bad ones. Both may be correct as far as compiling and when run, but bad code can present some problems in development, debugging, and modifying. In the workforce, no matter how well your program runs, someone will have to read and/or alter your code at some point. They may have to add new features, correct a rare bug, or just want to read it to understand how it works. Similarly, you will have to read someone else's code to do the same thing. Everyone will get along a whole lot better if the code is readable and understandable.

First, let's look at some bad code.

Katrina's Bad Code        Horrible horrible code!

So what's wrong with it?


// Kat

The comment header: The comment header gives no real information. It has my nickname and that's it. Not everyone knows your nickname, first name, or even your whole name. Put a full comment header there to people know who you are. For class, that's your name, class section, file name, date, and description. For work, it may be your name, employee number, and department number. Something to identify you and your work.


char mlevel;          // this is a valid char

Obvious comments: Next to the first variable declaration you see a comment: //this is a valid char. Avoid stating the obvious. As a reader, I know it's a char. I can see right there it is declared as a char. You don't need to tell me that. And of course it is valid. If it wasn't, it wouldn't compile.


double r, g, c, peoplewhoareadults;

Below the obvious char you see: double r, g, c, peoplewhoareadults; What do those mean?? Make your variable names meaningful. Make sure whoever reads it, including yourself, knows what a variable is being used for. But don't overdo it. Really long names make it annoying to write as well. So these variables should be things like: rainfall, gaslevel, adults. Also note that these were declared as doubles. Why would you need so much space for this? And how can you have a fraction of a person? Your variable types should make sense.

Another note about variables. It is a good habit to initialize variables to a number, usually 0. This way if you use the variable without setting it, it has a good value in it and not whatever gibberish was in memory. And do NOT list variables that are not constant before main.


cout << "Hi" << endl;
cin >> name >> endl;
cin >> r >> endl;
cin >> g >> endl;
cin >> c >> endl;
cin >> peoplewhoareadults >> endl;
cin >> chalmers >> endl;

After the variables you see a bunch of cin statements. First, it is a common mistake to put a >>endl at the end of a cin statement. This makes no sense; DON'T DO IT. Also, we obviously want to read in user input, but do we need all of this? And no prompts? Imagine yourself as the user of this program. It pops up and just says hi, not what it is or what it is doing, just hi, and starts flashing for input. It doesn't tell you what it is wanting you to input. Frustrating right? Prompt users for data. Also, only prompt for what you need when you need it. A lot of cases in this program, we don't need to know how many children and adults there are, so why ask for it everytime you run the program? It would be comparable to logging into Blackboard and being prompted for your gpa, each class grade, student number, username, and pass every time. Why?? So only ask for what you need, when you need it and prompt so the user knows what they are entering. And form a greeting to introduce your program. Just a title.


cout << "What is your motivation?\n"
<< " a. Amazingly Motivated\n"
<< " b. Basicly a good worker\n"
<< " c. Can't get good help no more\n"
<< " d. Don't plan on work from me\n"
<< " e. Elevated Slothfullness \n\n"
<< "Enter the letter of your choice: ";
cin >> mlevel;

if(mlevel=='a')        // this is an if statement

Anticipate user interaction. In this code, we give options and just ask them to enter a letter. When handling the option, we only accept lower case letters. Users however are not so simple. They may look at the statement and enter a capital letter. They may enter a number instead. We have to handle this. When prompting for input, guide the user to what you want. You may add a statement that says (lower case only). You may include capitals in your options: if(mlevel=='a' || mlevel=='A') And don't forget to check user input to make sure it's valid before continuing.


if(mlevel=='a') // this is an if statement
if(g<=5) // checks if g <= 5
cout << "burn books" << endl;
cout << "clean the bathroom" << endl;
{ .........

The first problem with this code is spacing/indentation. Keep your code indented with 2 spaces. Code that has no indentation is very hard to read. You will lose track of semicolons, brackets, etc. Nobody wants to open code and see this. On the other hand, keep your indentation consistent with 2 spaces. Indentation that goes all over the place is just as bad. Use copy and paste tools with editors carefully as some will really mess up your indentation. If you use a program like Notepad++, make sure it is set to C++. If you don't, it will create spacing/indentation issues. NEVER use Word! The second problem is the lack of use of constants. 0.5 and 5 were numbers given in an assignment. We could decide we want to change these numbers in the future, but for now they are not going to change. Make them constants. That way if we do decide to change them, we only have to change the number in one place at the beginning of the program instead of combing through thousands of pages of code looking for each instance of that number and hoping it is not a different use with the same value. Always list constants before your other variables and capitalize them!


cout << "Do laps in the tractor" << endl;
// This is a comment right in the middle that says I stayed up too late and didn't do my homework
// and now my code looks horrible. Large paragraphs of comments makes your code harder to read
// try using short statements to briefly explain what a block of code is actually doing instead
// of paragraphs that state nothing really important.
else if(mlevel=='b')

Keep comments short and sweet. We only want to know what that particular block of code does. We don't need to know your life story on how you created that code. Large paragraphs throughout your code make it hard to read. Make your comments brief and to the point.

Now let's look at the same program with good code practices.

Josh's Good Code

If you have any questions, contact your professor or TA.


Written by Katrina Ward, edited by Clayton Price