Does this simple C++ program need any further improvement?

[Armond Anand]

Hello guys, first of all I'm a complete noob when it comes to programming. So please forgive me if I unwillingly say something stupid. Now this is my first program written in C++ that I chose to make public in order to receive any suggestion as to whether or not it needs any further improvement, or whether I'm using any forbidden code or not.

I'm asking because I initially put a lot of GOTO codes in the program but later found out that using GOTO was evil (and that I could be eaten alive by a dinosaur). So 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.

Here's the code. Please let me know if you still see any evil or forbidden code that you expert programmers don't like. Thanks a lot in advance:

#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;
}

[toum]

The conditions of your do/while loops are not written correctly.
The line while (y_n_decision == "y" || "n") is understood by your compiler as
while ( (y_n_decision == "y") || ("n") ).
I doubt that's what you intended.
You have to repeat the comparison instruction as many times as there are comparisons to make: while (y_n_decision == "y" || y_n_decision == "n").

It's a matter of taste, but if you're a beginner in C++ I strongly encourage you not to use implicit conversions between C strings and C++ strings.
Making the conversions explicit will make your intentions clearer to the reader of your code and to the compiler, and will hopefully prevent some hard to diagnose bugs.
To make the conversion explicit, simply replace while (y_n_decision == "y" || y_n_decision == "n") with while (y_n_decision == string("y") || y_n_decision == string("n") )

[Armond Anand]

@toum Thanks so much!!

[Lowest0ne]

you are also looking for "n" when you asked for a "q"

[cire]

It's a matter of taste, but if you're a beginner in C++ I strongly encourage you not to use implicit conversions between C strings and C++ strings. [...]
To make the conversion explicit, simply replace while (y_n_decision == "y" || y_n_decision == "n") with while (y_n_decision == string("y") || y_n_decision == string("n") )

There are no conversions in the first version. All you're doing with the second is causing the unnecessary construction of string objects. I would encourage the OP to use the explicit conversions between C and C++ strings in other situations (again, there is no conversion in this one) as it makes your code easier to read.

[Armond Anand]

Wow thank you so much guys! I didn't expect so many responses! I really appreciate all your suggestions!

Okay I just simplified it a bit. The following code below will behave exactly the same as the previous one and I hope you guys will find it a little easier to read this time around. I fully admit that the previous code was totally garbage, specially with all those flaws @LowestOne found out. There seems to be a little debate between @cire and @toum about 'string conversions', something I got no clue of at the moment. But thanks anyways guys :-) The more I'm learning C++ and programming, the more I'm realizing how crucial it is to write readable and maintainable code.

#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;
}

[cire]

There seems to be a little debate between @cire and @toum about 'string conversions'

Toum made the assumption that the comparison operators are not overloaded for char*/string interaction. They are. It would be kind of silly for them not to be.

[toum]

Toum made the assumption that the comparison operators are not overloaded for char*/string interaction.

I did not. What I said was slightly incorrect but easier to explain to a beginner.

I really think it's a bad idea to let C++ beginners directly manipulate C style strings. You can be sure that if you ask Armond Anand to rewrite his code using only C strings, the only thing he will change is the variables' type. The "unnecessary construction of string objects" would at least hint that things might not be that simple.

[cire]

I did not. What I said was slightly incorrect but easier to explain to a beginner.

Correct and incorrect are not right and wrong. A thing is either correct or not. That it's easier to supply an incorrect explanation than a correct explanation... who would've thunk it?

The "unnecessary construction of string objects" would at least hint that things might not be that simple

Why not go a step further and just use std::strcmp from the start so when the inevitable happens it'll be obvious what to do?