2
\$\begingroup\$

I'm new to programming and even more new to C++. Anyway, I started to watch and read some tutorials and resources I found on the web.

I created this program to offer the user a coffee, expecting an answer of O (for "oui") or N (for "non"):

#include <iostream>
#include <string>

using namespace std;
int main()
{
        char cafe('G'); //Initialisation de la variable cafe

        while (cafe != 'O' && cafe != 'N' ) // Boucle: tant que l'utilsateur ne répond pas par O ou N, ça recommence.
        {

            cout << "Veux-tu du café ? O/N" << endl;

            cin >> cafe;  // input de l'utilisateur.

            if (cafe != 'O' && cafe != 'N')
                {
                    cout << "Merci de répondre par O ou par N." << endl;
                }

            else if(cafe == 'O')
                {
                    cout << "Regalez-vous !" << endl;
                }

            else if(cafe == 'N')
                {
                    cout << "Tant pis, a la prochaine !" << endl;
                }

        }
return 0;
}

One thing I dislike about it is that if the user enters a nonsense string like HJEHUEUHUEOOEL, then we'll get a prompt for each of those characters until O or N is found.

\$\endgroup\$
3
  • \$\begingroup\$ Welcome to Code Review. The current question title and description, which state your concerns about the code, apply to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. The description on the other hand should state the goals in more detail. Please see How to Ask for examples, and edit the title and description accordingly. \$\endgroup\$
    – Zeta
    Commented Jan 30, 2019 at 8:51
  • \$\begingroup\$ Thanks to both of you for helping me on the site. I should have read more before posting, sorry. I'm going to edit the code right now. \$\endgroup\$
    – COGNER
    Commented Jan 30, 2019 at 12:54
  • \$\begingroup\$ Concerning the fact that user may enter too many characters, one possibility is to read a string (with std::getline) and to check the value of the first entered character of the string only. \$\endgroup\$
    – Damien
    Commented Jan 30, 2019 at 13:30

1 Answer 1

4
\$\begingroup\$

Includes

We include <string> but don't use anything from it, so that line can be removed.

Namespace std

Avoid using namespace std - it won't hurt in simple programs, but it's a bad habit that could subtly change the meaning of programs. Import just the names you need, in the smallest reasonable scope, or just get used to writing std:: - it's intentionally very short!

Indentation

Pick a common indentation style, and use it consistently (perhaps that's a problem of how you've copied the code into the question, rather than a fault in the code itself?).

Always test input operations

The reading of a character with >> can fail - for example, if the input stream is closed.

Prefer do-while

Our loop should run at least once. do-while expresses that better than while, and means that we don't need to initialise cafe outside the loop.

Rearrange the if

We can re-order the if so that our current first case becomes the else clause:

    if (cafe == 'O') {
        std::cout << "Regalez-vous !" << std::endl;
    } else if (cafe == 'N') {
        std::cout << "Tant pis, a la prochaine !" << std::endl;
    } else {
        std::cout << "Merci de répondre par O ou par N." << std::endl;
    }

Consider switch

Any time we have different behaviours for particular values of a variable, we might use a switch statement instead of chained if/else if statements.

Exit early from the loop

We can break or return out of the loop in the success cases, rather than testing against O or N again. In this case, the loop can be indefinite (while (true)) and that's conventionally written as a while rather than do-while, so we change that back again.

Accept lower case letters

Allow the user to input o instead of O, or n instead of N. Users are simple creatures and will expect the program to cope with their assumptions.

Discard the rest of the input line

Something alluded to in the description: if the user enters a nonsense word, then each character in the word will be tested. We could ignore everything up to the next newline, like this:

        std::cin.ignore(999 , '\n');

(the 999 is just a "large" number of characters to ignore; we don't expect that many from one user line).


Improved code

#include <iostream>

int main()
{    
    while (true) {
        std::cout << "Veux-tu du café ? O/N" << std::endl;
        char cafe;
        std::cin >> cafe;
        if (!std::cin) {
            std::cerr << "Read error!\n";
            return 1;
        }
        // ignore rest of line
        std::cin.ignore(999 , '\n');

        switch (cafe) {
        case 'O':
        case 'o':
            std::cout << "Regalez-vous !" << std::endl;
            return 0;
        case 'N':
        case 'n':
            std::cout << "Tant pis, a la prochaine !" << std::endl;
            return 0;
        default:
            std::cout << "Merci de répondre par O ou par N." << std::endl;
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Hey ! Thank you so much for the answer. That's so complete ! I will try this out. \$\endgroup\$
    – COGNER
    Commented Jan 30, 2019 at 13:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.