"Favorite food" program

Question

This is my first program written in C++. I chose to make this public in order to receive suggestions on further improvements and to know if I'm using any "forbidden" code.

I'm asking because I initially put a lot of gotos in the program, but later found out that using goto was "evil." Therefore, after a lot of trial and error, I removed them all and replaced them with a couple of do-while loops. I must say using do-while makes the final code much easier to read.

Please let me know if you still see any "forbidden" code.

#include <iostream>
#include <string>

using namespace std;

int main () {

    string name, favfood, age, y_n_decision;

    do {

        cout << "welcome to my program in c++" << endl;
        cout << "Please enter your name" << endl;
        getline (cin, name);

        cout << "enter your favourite food" << endl;
        getline (cin, favfood);
        cout << "awesome! " << favfood << " is my favourite food too" << endl;

        cout << "enter your age" << endl;
        getline (cin, age);

        cout << " so your name is " << name << " and your fav food is " 
        << favfood << " and your age is " << age << endl;

        cout << "please press Y to restart, or Q to quit" << endl;

        do {

            getline (cin, y_n_decision);

            if (y_n_decision == "q") {
                cout << "quitting now" << endl;
                return 0;
            }

            else if (y_n_decision == "y") {
                cout << "nice! you typed y. Exiting current loop and returning to int main() aka the beginning." << endl;
                break;
            }

            else {
                cout << "you didn't type either y or q. YOU MUST PRESS Y OR Q OR I WILL PROMPT U FOREVER!!" << endl;
            }

        } while (y_n_decision == "y" || "n");

    } while (y_n_decision == "y" || "n");

    return 0;
}

EDITED:

#include <iostream>
#include <string>

using std::cout;
using std::endl;
using std::string;
using std::cin;


int main () {

    string name, favfood, age; 
    string y_n_decision;

    while (y_n_decision != "y" || y_n_decision != "q") {
        cout << "welcome to my program in c++" << endl;
        cout << "Please enter your name" << endl;
        getline (cin, name);

        cout << "enter your favourite food" << endl;
        getline (cin, favfood);
        cout << "awesome! " << favfood << " is my favourite food too" << endl;

        cout << "enter your age" << endl;
        getline (cin, age);

        cout << " so your name is " << name << " and your fav food is " 
        << favfood << " and your age is " << age << endl;

        cout << "please press y to restart, or q to quit" << endl;

        while (y_n_decision != "y" || y_n_decision != "q") {

            getline (cin, y_n_decision);


            if (y_n_decision == "q") {
                cout << "quitting now" << endl;
                return 0;
            }

            else if (y_n_decision == "y") {
                cout << "nice! you typed y. Exiting current loop and returning to int main() aka the beginning." << endl;
                break;
            }

            else { 
                cout << "you didn't type either y or q. YOU MUST PRESS y OR q OR I WILL PROMPT U FOREVER!!" << endl;
            }
        } 

    }
    return 0;
}

Answer 1

Armond, the main thing you should do is to learn about functions. Putting everything in main is not practicable beyond very small programs. And nested loops are generally a bad idea too. So for example you might extract the content of the first loop into a function and extract the second loop into another function.

So you would get something like:

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    return 0;
}

Also note that while (true) {...} is more commonly used than do {... } while (true); although do{} while() loops clearly have their uses.

---

EDIT

To be honest, I like the edited version less than the original. You are now testing the y_n_decision as the condition in both loops. And you are relying on the variable having been default-initialized. Moreover the loops are now while instead of do-while and are arguably more wrong now than they were before. A while (true) loop is preferable to your original do ... while (true) because the condition (true) is immediately visible at the top of the loop. But the new loops using while (condition) rely upon the initial value of the condition - and in your case you have to force that to be false by using an initializer that is not y/q. This is at best inelegant.

Here is how I might have coded what you are doing:

#include <iostream>
#include <string>
#include <ctype.h>

using std::cout;
using std::cin;
using std::endl;

static bool user_says_exit(void)
{
    std::string decision;
    do {
        cout << "Please press y to restart, or q to quit" << endl;
        getline(cin, decision);
    } while (decision != "y" && decision != "q");
    return (decision != "y");
}

static void user_interaction(void)
{
    std::string age;
    std::string favfood;
    std::string name;
    cout << "Welcome to my program in c++" << endl;
    cout << "Please enter your name" << endl;
    getline (cin, name);
    cout << "Enter your favourite food" << endl;
    getline (cin, favfood);
    cout << "Awesome! " << favfood << " is my favourite food too" << endl;
    cout << "Enter your age" << endl;
    getline (cin, age);
    cout << "So your name is " << name << ", your fav food is "
         << favfood << " and your age is " << age << endl;
}

int main(int argc, char **argv)
{
    do {
        user_interaction();
    } while (user_says_exit() == false);
    cout << "Quitting now" << endl;
    return 0;
}

Answer 2

Style

• Make sure that your code is consistent in the style. Choose whether you put the opening brace on the next line or on the current line and stick to it.

User interface

• If you expect 'y' or 'q' from the user, don't ask him to press 'Y' or 'Q'.
• In your case, this is probably more some debug logs than a proper user interface but as a rule, you probably should bother the user with implementation details in your user interface ("current loop", "int main()").

Code organisation

• Define your variables in the smallest possible scope.
• You shouldn't do using namespace std. SO question about this

Code flow

• At the moment, your code is checking y_n_decision != "y" || y_n_decision != "q" in different places. In order to see when it is true, let's try to see when it is actually false. This happens when the two sides or the || are false which is when y_n_decision is both "y" and "q". This cannot happen thus the condition is always true and not really useful. You can get rid of your checks making the code easier to read. (You should also take that chance to move the declaration of y_n_decision to the inside of the inner while now that we don't need it anywhere else anymore).

(To be honest, before diving into the code, I thought it would be a bit harder to clean the whole thing but once this obviously-useless checks are removed, everything is already much clearer).