patterncppModerate
"Who are you, and where do you live?" — a Q&A exercise
Viewed 0 times
youarewhowhereexerciseliveand
Problem
This is my first attempt at a programme. Is it good form? Does it follow best practice? Or am I completely off the mark?
#include "stdafx.h"
#include
#include
#include
using namespace std;
string a;
string b;
string enterName()
{
while (true)
{
cout 0 && x < 95)
return x;
cout << "That is not possible, put your real age please." << endl;
}
}
string address()
{
while (true)
{
cout << "Where do you live " << a << "?: ";
string c;
getline(cin, c);
if (c == "")
cout << "You must live somewhere." << endl;
if (c != "")
return c;
}
}
int _tmain(int argc, _TCHAR* argv[])
{
system("Color 1A");
string a = enterName();
cout << endl;
int y = verifyAge();
cout << endl;
string c = address();
cout << endl;
system("cls");
cout << endl << endl << endl;
cout << "So your name is " << a << " you are " << y << " years old"
<< " and you live in " << c << endl << endl << endl;
cout << "Press any key to exit";
cin.get();
return 0;
}Solution
I have found a couple of things that could help you improve your code.
Don't abuse
Putting
Avoid the use of global variables
I see that
Use meaningful variable names
The variable names
Don't use
There are two reasons not to use
General portability
This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files
Fix your formatting
There are abundant examples here of C++ code that is well formatted. This code has peculiar indentation that makes it difficult to tell when a function begins and ends. Fixing that would help.
Make it clear when loops end
The code currently uses
Variable range checking
Your
In all, the program actually functions as intended, so you are indeed on your way.
Don't abuse
using namespace stdPutting
using namespace std at the top of every program is a bad habit that you'd do well to avoid. Avoid the use of global variables
I see that
a and b are declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. It's a bit strange because you have done that with variable c in your _tmain function. Use meaningful variable names
The variable names
a, b, and c are not at all descriptive. Better names might be name, age and address. Doing so makes your code easier to read, understand and maintain.Don't use
system("cls")There are two reasons not to use
system("cls") or system("Color 1A"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or Color, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a separate functions cls() and color() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. General portability
This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files
#include "stdafx.h" and #include and use the standard int main() rather than the Windows-only _tmain. See this stackoverflow question for a longer explanation.Fix your formatting
There are abundant examples here of C++ code that is well formatted. This code has peculiar indentation that makes it difficult to tell when a function begins and ends. Fixing that would help.
Make it clear when loops end
The code currently uses
while (true) in a number of places, but it doesn't really loop infinitely. It's better, generally, to have the loop exit condition explicitly stated. For example we could rewrite your enterName() function:string enterName()
{
do
{
cout << "what is your name?: ";
getline(cin, a);
if (a == "")
cout << "Let's try that again." << endl;
} while (a == "");
return a;
}Variable range checking
Your
verifyAge() routine seems to think that it's impossible for people to be 95 year old or older, but I personally know people that old who still use a computer. A number of 130 might be a safer bet to use as an upper range.In all, the program actually functions as intended, so you are indeed on your way.
Code Snippets
string enterName()
{
do
{
cout << "what is your name?: ";
getline(cin, a);
if (a == "")
cout << "Let's try that again." << endl;
} while (a == "");
return a;
}Context
StackExchange Code Review Q#63472, answer score: 16
Revisions (0)
No revisions yet.