Please Review My Number Guessing Program

Hi, I just recently started learning C++, and the objective of my last project was to have my program asks the user to think of a number then it would attempt to guess that number. It performs fine, but I would like opinions on how to make it better and cleaner or just any advice in general.

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
44
45
46
47
48
49
50
51
52
53
54
55
56
#include <iostream>
using namespace std;

int main()
{
    cout <<"Think of a number between 1 and 100. ";
    
    int highest = 100, lowest = 0, attempts = 1;
    
    int guess = lowest + ((highest - lowest) * 0.5);
    cout << endl;
    
    cout << "I guess " << guess << ". Am I right?" << endl;
    cout <<  "’q’ to quit, ‘y’ if correct, ‘h’ if too high, ‘l’ if too low. " << endl;
    char UserChar = '\0';
    cin >> UserChar;
    
    while(UserChar != 'q' && UserChar != 'Q')
    {
        if (UserChar == 'h' || UserChar == 'H')
        {
            highest = guess;
            guess = lowest + ((highest - lowest) * 0.5);
            cout << "I guess " << guess << ". Am I right?" << endl;
            cout <<  "’q’ to quit, ‘y’ if correct, ‘h’ if too high, ‘l’ if too     low. " << endl;
            cin >> UserChar;
            attempts++;
        }
    
        else if (UserChar == 'l' || UserChar == 'L')
        {
            lowest = guess;
            guess = lowest + ((highest - lowest) * 0.5);
            cout << "I guess " << guess << ". Am I right?" << endl;
            cout <<  "’q’ to quit, ‘y’ if correct, ‘h’ if too high, ‘l’ if too low. " << endl;
            cin >> UserChar;
            attempts++;
        }
    
        else if (UserChar == 'y' || UserChar == 'Y')
        {
            cout << "I guessed it in " << attempts << " attempts." << endl;
            exit(attempts);
        }
        else
        {
            cout << "I didn't understand that respond." << endl;
            exit(attempts);
            
        }
    }
    
    cout << "You quit. Bye!" << endl;
    
    return attempts;
}
I would like opinions on how to make it better and cleaner or just any advice in general.

Well, your code looks pretty clean to me. But I can see a bit of repetition which you should be able to eliminate.

Bug!

When I chose 0 it successfully guessed it but not when I chose 100 (99 is the maximum it tried.) So the range handled is slightly off.

Risk

You might also want to think about using an alternative to cin >> UserChar; to get the user's response as it allows multiple entries to be entered at the same time, like this:

Think of a number between 1 and 100.
I guess 50. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.
hhh
I guess 25. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 12. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 6. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
lhl
I guess 9. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 8. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.
h l
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.
h l h l
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too     low.
I guess 7. Am I right?
'q' to quit, 'y' if correct, 'h' if too high, 'l' if too low.


(Actually, looking at this output I can see I managed to confuse the selection of the next guess by inadvertently lying about my number. The answer should have been 'y' to 8 (I said 6 was low, 9 was high, and 7 was low) but it ended up fixating on 7. Maybe you could tweak the code so it spots when it just has to be a particular number?)

And

1. I don't really like to see exit() used. You should be able to have a single return call from main() in this case with a slight tweak to your code.
2. It's a bit mean to exit if the use enters an invalid response?
3. A switch statement might be better than the if-else if-... chain.
4. Have you come across the standard function tolower() yet?
5. A do-while loop might work better than a while loop here.
6. initializing attempts to 1 and lowest to 0 reads a bit strangely to me?
7. Returning attempts from main is non-standard. A program should return 0 on successful completion.
8. Why is the endl on line 11 rather than at the end of line 6 ??

But the main issues are the repetition of code and the bug.

Andy

PS It is seen to be better form to not use using namespace std;, but I wouldn't worry about that for now.
Last edited on
Thanks for the great feedback, Andy! It was really helpful. :) I fixed everything that you listed, and it looks and performs so much better now. I tried to avoid confusing the program by adjusting the values of highest and lowest every time there's a 1 or 0 difference, and although it worked, it still messed up the program pretty badly. So, instead, I just reset the value of highest to 101 and lowest to 0. I pasted the code below for you to see. Is there another way to fix this to improve program performance?

Any other constructive comments on my revised code are welcomed by the way.

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
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102

#include <iostream>
#include <string>

using namespace std;


int GuessNum(int L, int H)
{
    int guess = L + ((H - L) * 0.5);

    return guess;
}


char MultInputCheck(string SomeChar)
{
    
    if (SomeChar.length() == 1)
    {
        SomeChar[0] = tolower(SomeChar[0]); //makes everything lowercase
        return SomeChar[0];
    }
    else
    {
        return 'm'; //to make program goes straight to default in switch-case statement
    }
    
}

int main()
{
    cout <<"Think of a number between 1 and 100."<< endl << endl;
    
    int highest = 101, lowest = 0, attempts = 0;
    
    char UserChar  = '\0';
    string UserInput; // stores user's input as string to check if it's more than 1 character in MulInputCheck function

    
    do
    {
        
        cout << "I guess " << GuessNum(lowest, highest) << ". Am I right? " << endl;
        cout <<  "’q’ to quit, ‘y’ if correct, ‘h’ if too high, ‘l’ if too low. " << endl;
        getline(cin, UserInput);
        
        UserChar = MultInputCheck(UserInput);
        attempts++;
        
        
        switch(UserChar)
            {
                case 'h':
                    highest = GuessNum(lowest, highest);
                    
                    if (highest == lowest)
                        lowest = 0;
                    else if (highest - 1 == lowest)
                        lowest = 0;
                    
                    break;
                
                case 'l':
                    lowest = GuessNum(lowest, highest);
                    
                    if (highest == lowest)
                        highest = 101;
                    else if (highest - 1 == lowest)
                        highest = 101;
                    
                    break;
    
                case 'y':
                    cout << "I guessed it in " << attempts << " attempts." << endl;
                    break;
                
                default:
                    if (UserChar == 'q')
                        break;
                
                    else
                    {
                        cout << "I didn't understand that respond. Would you like to try again? 'q' to quit, anything else to continue. " << endl;
                        getline(cin, UserInput);
                        UserChar = MultInputCheck(UserInput);
                        
                        if (UserChar != 'q')
                            continue;
                    }
            }
        
    } while(UserChar != 'q' && UserChar != 'y');
    

    if (UserChar == 'q')
        cout << "You quit. Bye!" << endl;
    
    return 0;
}

Well, the MultInputCheck looks like a good idea. It would be even better if it could handle leading and trailing quite space!

But the bug isn't completely fixed -- I can guess up to 100 now but can still guess down to 0 when you say the lowest number should be 1.

I don't get the code on line 57-60 and 67-70

Line 21-22 looks more complicated than necessary.

The handling of 'q' in the switch statement doesn't look quite normal?

GuessNum() could maybe be called fewer times?

I see that if a bad response is given the attempt count still increases. Is this intended?

Oh yeah, and it's not so tidy this time round.

Andy
Last edited on
Topic archived. No new replies allowed.