Embedded Files
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:
// Kat
#include <iostream>
using namespace std;
int main()
{
char mlevel; // this is a valid char
bool chalmers;
string name;
double r, g, c, peoplewhoareadults;
cout << "Hi" << endl;
cin >> name >> endl;
cin >> r >> endl;
cin >> g >> endl;
cin >> c >> endl;
cin >> peoplewhoareadults >> endl;
cin >> chalmers >> endl;
cout << "What is your motivation?n" << " a. Amazingly Motivatedn" << " b. Basicly a good workern"
<< " c. Can't get good help no moren" << " d. Don't plan on work from men"
<< " e. Elevated Slothfullness nn" << "Enter the letter of your choice: ";
cin >> mlevel;
if (mlevel == 'a') // this is an if statement
{
if (r >= 0.5) {
if (g <= 5) // checks if g <= 5
{
cout << "burn books" << endl;
} else {
cout << "clean the bathroom" << endl;
}
} else {
if (g <= 5) {
cout << "Go get more g" << endl;
} else if (g >= 5 && g < 10) {
cout << "Mow grass" << endl;
} else
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') {
if (chalmers == 1 || peoplewhoareadults > c) {
cout << "Scrub floors on hands and knees" << endl;
} else {
cout << "Mop the floor." << endl;
}
} else if (mlevel == 'c') {
if (r <= 1.5) {
cout << "Lean on rake." << endl;
} else
cout << "Lean on broom inside" << end;
} else if (mlevel == 'e') {
cout << "Stay in bed" << endl;
}
return 0;
}
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(r>=0.5)
{
if(g<=5) // checks if g <= 5
{
cout << "burn books" << endl;
}
else
{
cout << "clean the bathroom" << endl;
}
}
else
{ .........
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!
else
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:
//Programmer: Josh Wilkerson Date: 2011/09/18
//File: workDecider.cpp Class: CS53
//Purpose: This program helps groundskeeper Willie select
// the appropriate amount of work to do, given a
// level of motivation
#include<iostream>
using namespace std;
const float LEV_A_RAIN_THRESH = 0.5;
const int LEV_A_LOW_GAS = 5;
const int LEV_A_HIGH_GAS = 10;
const float LEV_C_RAIN_THRESH = 1.5;
const float LEV_D_RAIN_THRESH = 0.0;
int main()
{
char motivLev = 0, chalmersPresent = 0;
string userName = "";
float rainLev = 0;
int gasLev = 0, numChildren = 0, numAdults = 0;
//welcome and prompt the user for their name
cout << "\nWelcome to the Work-Avoider 2000\n"
<< "--------------------------------\n"
<< "Please enter your name: ";
cin >> userName;
//put the riff-raff in their place
//(NOTE: this is not part of the assignment, per se; but is obviously
//a necesary addition)
if(userName == "ClaytonPrice" || userName == "TommySzalapski")
{
cout << "\nOh come on, we both know you aren't going to do work regardless "
<< "of what I say.\n"
<< "Fine, we'll go through the motions anyway" << endl;
}
//prompt for the motivation level
cout << "\nHow motivated are you today, " << userName << "?\n"
<< " a) Amazingly motivated\n"
<< " b) Basically 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 corresponding to your motivation level: ";
cin >> motivLev;
//determine what response to give, allowing the user to use upper or lower
//case for their motivation level
/////////////
//motivation level A
/////////////
if(motivLev == 'a' || motivLev == 'A')
{
cout << "\nWell are't we a hard worker, very impressive.\n"
<< "Tell me, " << userName << ", how many inches did it rain last "
<< "night?\n";
cin >> rainLev;
cout << "Ok, and how many gallons of gas do you have on hand (to the "
<< "nearest gallon, please)?\n";
cin >> gasLev;
//if it rained more than the rain threshold for this level
cout << endl << userName << ", you should ";
if(rainLev >= LEV_A_RAIN_THRESH)
{
cout << (gasLev <= LEV_A_LOW_GAS ?
"burn some of those ridiculous text books from the 1940's"
:
"go purify that bathroom with righteous flames of cleansing") << endl;
}
//if it rained less than the rain threshold for this level
else
{
//if we don't have much gas
if(gasLev < LEV_A_LOW_GAS)
cout << "go get more gas, then ask me what to do again" << endl;
//we have a moderate amount of gas
else if(gasLev >= LEV_A_LOW_GAS && gasLev < LEV_A_HIGH_GAS)
cout << "rev up the mower and take care of that overgrown yard" << endl;
//we have a lot of gas
else
{
cout << "fill up the biggest tractor you have and do some laps where\n"
<< "everyone can admire your impressive work ethic" << endl;
}
}
}
/////////////
//motivation level B
/////////////
else if(motivLev == 'b' || motivLev == 'B')
{
cout << "\nOk, pretty mortivated then.\n"
<< "In that case, is Superintendent Chalmers where he can see you?\n"
<< "(y or n, please): ";
cin >> chalmersPresent;
//if chalmers is present then we clean the floor on hands and knees
if(chalmersPresent == 'y' || chalmersPresent == 'Y')
{
cout << endl << userName << ", you should get on your hands and knees and"
<< " scrub that floor, an impressed boss is\n"
<< "a happy boss!" << endl;
}
//otherwise we see how many adults and children are present
else
{
cout << "Ok, the boss isn't watching. How many adults are around, then?\n";
cin >> numAdults;
cout << "And how many children are around?\n";
cin >> numChildren;
//if there are more adults than children, clean on hands and knees, mop
//otherwise
cout << endl << userName << ", you should ";
cout << (numAdults >= numChildren ?
"get some word of mouth spreading, scrub the floor on hands and knees"
:
"just mop the floor")
<< endl;
}
}
/////////////
//motivation level C
/////////////
else if(motivLev == 'c' || motivLev == 'C')
{
cout << "\nOk, so you just want to look busy; fair enough.\n"
<< "How many inches did it rain last night?\n";
cin >> rainLev;
cout << endl << userName << ", you should ";
//if it rained less than this level's rain threshold lean on a rake,
//otherwise lean on a broom inside
cout << (rainLev <= LEV_C_RAIN_THRESH ?
"go get your rake and lean on it"
:
"go inside and lean on your broom")
<< ", looking as annoyed as possible\n"
<< "(makes you look busier)" << endl;
}
/////////////
//motivation level D
/////////////
else if(motivLev == 'd' || motivLev == 'D')
{
cout << "\nOk, motivated enough to go to work, but not motivated enough to"
<< " deal with anyone.\n"
<< "How many inches did it rain last night?\n";
cin >> rainLev;
cout << endl << userName << ", you should go hide in ";
//if it rained less than this level's rain threshold hide in a hedge,
//otherwise hide in the toolshed
cout << (rainLev <= LEV_D_RAIN_THRESH ?
"a hedge"
:
"the toolshed")
<< endl;
}
/////////////
//motivation level E
/////////////
else if(motivLev == 'e' || motivLev == 'E')
{
//NOTE: this output uses escape sequences (the \"'s), if you haven't learned about
//these by the time you read this, you will soon
cout << "\n\"I don't always stay in bed and skip work, but when I do it's because\n"
<< "a computer program told me to\"\n"
<< " --The most unmotivated man in the world\n\n"
<< "Enjoy your \"sick day\", " << userName << endl;
}
/////////////
//unknown motivation level
/////////////
else
{
cout << "ERROR 87328047:\n"
<< "UNRECOGNIZED MOTIVATION LEVEL\n"
<< "TERMINATING" << endl;
}
cout << "\n--------------------------------\n"
<< "Thank you for using the Work-Avoider 2000\n" << endl;
return 0;
}
If you have any questions, contact your professor or TA.
Page updated
Report abuse