Is my compiler messed up or am I making a mistake?

I have this:


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
char choice;
cout << "Enter A to add an item, D to delete an item, S to search for an item, L for the length of the list, P to print list, or E to exit:"<< endl;
    cin >> choice;

    while ((choice=!'E')||(choice!='e'))
    {
      {
        if ((choice=='A')||(choice=='a'))
        {
            cout << "Enter the integer you want to add:" << endl;
            cin >> userEnteredInt;
            addItem(userEnteredInt, intList);
        }
        else if ((choice=='D')||(choice=='d'))
        {
            cout << "Enter the integer you wish to delete:" << endl;
            cin >> userEnteredInt;
            deleteItem(userEnteredInt, intList);
        }
        else if((choice=='S')||(choice=='s'))
        {
            cout << "Enter the integer you wish to search for:" << endl;
            cin >> userEnteredInt;
            searchItem(userEnteredInt, intList);
        }
        else if((choice=='L')||(choice=='l'))
        {
            lengthList(intList);
        }
        else if((choice=='P')||(choice=='p'))
        {
            printList(intList);
        }
        else
            cout <<"Invalid Entry" <<endl;
      }
    cout << "Enter A to add an item, D to delete an item, S to search for an item, L for the length of the list, P to print list, or E to exit:"<< endl;
    cin >> choice;
    }


For every letter I enter, it return "invalid entry". Am I doing something wrong, or is my compiler out of control? I have included string and iostream.
I very much doubt if your compiler is out of control.

I think you would be much better off using a switch statement for this.

You can either make use of the toupper function to make the testing easier or just put both lower / upper case options in the case clause:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
switch (choice) {
      case 'e':
      case 'E':
           //your code
     break;

     case 'a':
     case 'A':
           //your code
     break;

      default:
          //your code

}


If the amount of code in a case clause is getting too much then call a function from the case.

HTH

BTW You can build up cout statements in parts - there is no need to have all the code in 1 line, this is equivalent:

1
2
3
cout << "Enter A to add an item, D to delete an item,";
cout << " S to search for an item, L for the length of the list,";
cout << " P to print list, or E to exit:"<< endl;
Last edited on
 
while (choice =! 'E'  || choice != 'e')

The condition of the while loop is, from what I believe, erroneous. The not operator is performing an operation on the given character literal and then assigning that value to choice. Your program would function as expected if you were to switch the two operators (=, !).

 
while (choice != 'E'  || choice != 'e')
Last edited on
> Is my compiler messed up or am I making a mistake?

Start with the axiom: "my compiler is not messed up; I am making a mistake". And stick with it till you reach reductio ad absurdum.

In the long run, you would be saving yourself a huge amount of time.
Using a switch helps avoid the error pointed out by eagle-eye xhtmlx .

I think it looks a lot better as well. It is cleaner & easy to follow, and is easily scalable. And less error prone.

I always try to avoid testing twice for upper / lower case - it just makes the test expressions easier.
Topic archived. No new replies allowed.