Code Review

Here is a program I just started that asks for a name, and then asks the user if they are sure that is the name they want (eventually it will ask them to re enter their name if they choose no.) For now I have it working, and I just want advice on what I could do better and how to make it more efficient, because I really don't know what good code looks like, so help me out! Also I would like it if someone could tell me why the do while loop works, because I typed while (!valid) and it works but logically that doesn't make sense to me, because valid is initialized as false and stays false until it is true, so wouldn't that statement be saying do while (valid is not false) ? That doesn't make sense, shouldn't it be do while (valid is false) ? Why does the logical not make it work? Thanks!
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
#include <iostream>
#include <string>

void introduction();
bool validate(std::string);

int main()
{
	introduction();
}

void introduction()
{
	std::cout << "Hello my fair-weathered friend! And what mightest thy name be?\n";
	std::string name;
	std::cin >> name;
	
	std::cout << "Ah! Thine name is " << name << "? Art thou sure? (Y/N)";
	std::string yesOrNo;
	
	bool valid = false;

	do
	{
		std::cin >> yesOrNo;
		valid = validate(yesOrNo);
		if(valid == false)
			std::cout << "Please enter Y or N! Read the directions you insolent twerp!";
		
	} while(!valid);

	std::cout << "Grand!";
}

bool validate(std::string str)
{
	if(str == "Y" || str == "y" || str == "Yes" || str == "yes")
		return true;
	else if(str == "N" || str == "n" || str == "No" || str == "no")
		return false;
	else 
		return false;
}
http://codereview.stackexchange.com/ will help you.

1.main need return; compile will complain it, Never ignore warnings.
2.std cout need flush the buffer. You can do like this std::cout << std::endl;
3.Learn regex, it is complicated but interesting.
The while ( cond ) repeats the loop as long as the cond is true. The loop breaks, then cond becomes false.

As long as valid == false, the !valid == true and thus the loop repeats. When the valid changes to true, the !valid changes to false and the loop ends.

You do have a logical problem though. The loop ends only if the user chooses yes. There is no difference between writing 'no' or garbage. You should have two levels of test. The validation loop continues until the input is valid (yes or no). Outer test checks whether the valid input was yes or no.

Obviously, you don't want to repeat the lines 37 and 39, so the validate() should condense the answer somehow.
Hi,

You could make life easier for yourself by not asking for a std::string for yes/no questions. How many mixed combinations of y, e, s, Y, E, S are there ? Should you test for them all?

In my mind it would be better to restrict the input to a single char, then use either tolower or toupper functions to very simple logic. Have a look in the reference material at the top left of this page about those functions.

Hope all goes well :+)
Topic archived. No new replies allowed.