-2

I'm having some trouble with a small number generator thingy. Everything works the way it should, yet I'm just fixing errors. I'm extremely new to coding, started barely 2 weeks ago, so please excuse my lack of knowledge.

Here's my code:

cout << "\n\nPlease enter a number for one of the following:" << endl;

cout << "1: Completely random number generator (no specified limit)" << endl;

cout << "2: Number generated from x to x, you decide" << endl;

cout << "3: Number generated from 1 to 100" << endl;

cout << "4: Number generated from 1 to 10" << endl;

cout << "5: Random number generated with decimal\n\n" << endl;

int menuSelection;

cin >> menuSelection;
system("cls");

if (menuSelection != 1||2||3||4||5 ) // my problem lies here
{
    cout << "Please enter a valid selection from 1 to 5!" << endl;
    goto mainMenu;
}

Basically, I want my program to not screw up and crash when someone cheeky enters something that is not an integer, and to output that error.

m0ite
  • 3
  • 4
  • 3
    `menuSelection != 1||2||3||4||5` doesn't do what you think it does. – πάντα ῥεῖ Oct 30 '15 at 11:44
  • There are places when using `goto` is acceptable. Using it for a loop is not considered one of those uses. – Some programmer dude Oct 30 '15 at 11:45
  • Thanks, but what is wrong with it? I know it's lazy but do you mind explaining? – m0ite Oct 30 '15 at 11:46
  • πάντα ῥεῖ, I know that it obviously doesn't do what I think it does, but I had to throw something in there in order for people to understand my problem. That is not what I've come up with – m0ite Oct 30 '15 at 11:48
  • Your issue is the program does not understand what to compare the numbers proceeding 1 against. It should be `menuSelection != 1 || menuSelection != 2 || etc...` Alternatively you can just do a case statement they are much neater for this type of thing. You can then define a `default` to catch any case that doesn't match the cases you specified. – M Davies Oct 30 '15 at 11:49
  • @m0ite _"That is not what I've come up with"_ Always post your real code here. – πάντα ῥεῖ Oct 30 '15 at 11:51
  • Your first suggestion did not seem to work, my program still crashes when a letter is inputted, of course, I could ignore these problems, yet I want a solid program. – m0ite Oct 30 '15 at 11:54
  • @πάντα ῥεῖ I meant to say, I haven't come up with anything, yet I've tried everything, if I posted what I had "come up with" the condition would have been blank. – m0ite Oct 30 '15 at 11:56
  • 1
    I think you need to re-read the book on C++ programming – Ed Heal Oct 30 '15 at 12:07

3 Answers3

2

Instead of

if (menuSelection != 1||2||3||4||5 )

it is simpler to write

if (menuSelection <  1 || menuSelection > 5 )

As for this expression

menuSelection != 1||2||3||4||5

then there is used the so-called equality operator != and the logical OR operator ||

The last has a lower priority compared with the priority of the equality operator.So the expression looks like

( menuSelection != 1 )||2||3||4||5

The subexpression in the parentheses yields boolean true or false. Then its result is used in the subexpression with the operator || and operand 2. A non-zero integer is implicitly converted to boolean true

So in fact you have

( menuSelection != 1 )|| true || true || true ||true

that independing of the comparison menuSelection != 1 will always yield true

The more correct expression will look like

menuSelection != 1 || menuSelection != 2 || menuSelection != 3 || menuSelection != 4 || menuSelection != 5

But even this expression is wrong in the context of your program. You have to use logical AND operator && instead of the logacal OR operator ||.

menuSelection != 1 && menuSelection != 2 && menuSelection != 3 && menuSelection != 4 && menuSelection != 5
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Thanks for your input! Yet my problem remains the same. My program still crashes when an ASCII character is inputted. – m0ite Oct 30 '15 at 12:04
  • @m0ite You should check that the input was not failed. For example if ( !( cin >> menuSelection ) ) cin.ignore( std::numeric_limits::max() ); – Vlad from Moscow Oct 30 '15 at 12:09
  • That looks intimidating to a beginner like me, I suppose I'll pick it up as I go, but I'll try it. – m0ite Oct 30 '15 at 12:14
0

Actually, the condition which you want to implement is the following:

if (menuSelection != 1 && menuSelection != 2 && menuSelection != 3 && menuSelection != 4 && menuSelection != 5) // my problem lies here
{
    cout << "Please enter a valid selection from 1 to 5!" << endl;
    goto mainMenu;
}

However, it is not convenient. Since menuSelection is an integer, you can simply compare it:

if (menuSelection < 1 || menuSelection > 5) {
    cout << "Please enter a valid selection from 1 to 5!" << endl;
    goto mainMenu;
}

As the final word, I want to say: goto? Please, don't. That's not how you implement good C++ applications.

You can easily replace it with something like this:

cout << "\n\nPlease enter a number for one of the following:" << endl;
cout << "1: Completely random number generator (no specified limit)" << endl;
cout << "2: Number generated from x to x, you decide" << endl;
cout << "3: Number generated from 1 to 100" << endl;
cout << "4: Number generated from 1 to 10" << endl;
cout << "5: Random number generated with decimal\n\n" << endl;

int menuSelection;
cin >> menuSelection;

while (menuSelection < 1 || menuSelection > 5)
{
    cout << "Please enter a valid selection from 1 to 5!" << endl;
    cin >> menuSelection;
}

// Here, menuSelection is definitely between 1 and 5 inclusively

You can read a lot of information on this topic. For example, at StackOverflow:
'Goto' is this bad?

Community
  • 1
  • 1
Yeldar Kurmangaliyev
  • 33,467
  • 12
  • 59
  • 101
  • My problem remains the same. My program still crashes when an ASCII character is inputted. As I said in an above comment, what I wrote was not my final conclusion, yet I figured I had to write some bs code in there for people to understand what I want, hence why it looked yuck, even to me. I appreciate your input, though! – m0ite Oct 30 '15 at 12:06
  • Could I change `menuSelection` to a string? Then use what you've written? – m0ite Oct 30 '15 at 12:08
  • @Yeldar Kurmangaliyev Why is Russian your native language? I thought that it is Kazakh that is your native language. – Vlad from Moscow Oct 30 '15 at 15:22
  • @VladfromMoscow Good evening. I simply do not speak Kazakh, but I speak Russian since I was born. So, I can't call Kazakh my native language. Moreover, according to Ozhegov's dictionary, "native" is such language, which is acquired from parents instinctively without education, but not that one which belongs to my ethnic origin. Many Kazakhs speak Russian better than Kazakh, or do not speak the second one at all - especially, in the Northern regions :) I guess, we shouldn't chat and flame here. If you want to discuss it - there is a link to my VK profile in StackOverflow profile. – Yeldar Kurmangaliyev Oct 30 '15 at 18:53
0

I solved my initial problem by changing menuSelection to a string. Again, I apologize for my lack of knowledge, I'm trying my best. Thanks everyone for their kind input.

m0ite
  • 3
  • 4