Thread: can this program be made simple?? if yes pls help out!

Write a menu program that lets the user select from a list of options, and if the input is not one
of the options,reprint the list.
Is this program correct?..i can execute this program but can this code be made simple?

Code:

#include<iostream>
using namespace std;
int main()
{
int option;
    for(int option; ;)
    {


cout<<"1.apple"<<'\n'<<"2.mango"<<'\n'<<"3.grape"<<'\n'<<"4.orange"<<'\n'<<"5.banana"<<'\n';
cout<<"select any one of the option listed above(enter the number of your option):\n";
cin>>option;
    for(option;option<6;)
    {
        if(option==1)
        {
            cout<<"apple\n";
            break;
        }
        else if(option==2)
        {
            cout<<"mango\n";
            break;
        }
        else if(option==3)
        {
            cout<<"grape\n";
            break;
        }
        else if(option==4)
        {
            cout<<"orange\n";
            break;
        }
        else if(option==5)
        {
            cout<<"banana\n";
            break;
        }
        else
        {
            cout<<"enter the number again:\n";
        }
    }
}
}

It is good that you made some effort to indent your code, but you need to be consistent, e.g.,

Code:

#include<iostream>

using namespace std;

int main()
{
    int option;
    for (int option; ;)
    {
        cout << "1.apple" << '\n' << "2.mango" << '\n' << "3.grape" << '\n' << "4.orange" << '\n' << "5.banana" << '\n';
        cout << "select any one of the option listed above(enter the number of your option):\n";
        cin >> option;
        for (option; option < 6;)
        {
            if (option == 1)
            {
                cout << "apple\n";
                break;
            }
            else if (option == 2)
            {
                cout << "mango\n";
                break;
            }
            else if (option == 3)
            {
                cout << "grape\n";
                break;
            }
            else if (option == 4)
            {
                cout << "orange\n";
                break;
            }
            else if (option == 5)
            {
                cout << "banana\n";
                break;
            }
            else
            {
                cout << "enter the number again:\n";
            }
        }
    }
}

A few things that I observed:

• You declared option twice: once in the immediate scope of the main function and another time in the scope of the outer for loop. The rule of thumb is to declare variables near first use, so I would declare it in the scope of the for loop.
• Are you sure that you are supposed to loop forever instead of requesting for user input that will indicates the end of the program?
• The printing of the menu and prompt can be simplified to:
  
  Code:

  cout << "1.apple\n2.mango\n3.grape\n4.orange\n5.banana\n"
          "select any one of the option listed above(enter the number of your option):\n";

• Your inner for loop could be replaced by an if statement, in which case you could get rid of the break statements. Actually, I would just write the if-else chain that you wrote inside the inner for loop.
• Instead of the if-else chain, you could use a switch statement. (Though then the breaks go back in, but it is no big deal.)

I would put strings into array and then access string by index - so I do not need to modify code when adding the new option - only the array

Originally Posted by Guest

Is this recommendation mostly down to readability, or because of performance implications?

Both, though performance is not a concern here since the variable is of a built-in type, and in some cases where the variable is expensive to construct and cheap to modify minimally perhaps one would avoid declaring it within a loop instead.

It's probably better to use a do-while loop instead, which in addition to the array of strings would look something like this:

Code:

    // Initialize an array of strings for the options:
    std::string options[] = {/* options */};

    // Record the total number of strings in options array:
    int numOptions = sizeof(options) / sizeof(options[0]);

    // Declare a variable to hold user input:
    int userSelection = 0;
    do
    {
        // Print options with for loop, use the loop variable to number the options.
        // Ask user to choose a number associated with an option.
        // Record user selection into the variable 'userSelection'

    } // Keep loop going so long as user input is invalid:
    while (userSelection < 0 || userSelection >= numOptions);

    // Print the string in options that the user selected.

The do-while loop will allow you to get the users input at least once before needing to check the variable that is to hold the input. That makes the loop body a good place to put the messages you want displayed on invalid input.

Originally Posted by recklez

2.I'm not sure in the question it says "if the input is not one of the options,reprint the list." i couldn't get a single reprint of the menu so the infinite loop.

Well, is "exit" supposed to be one of the options? If not, then it looks like an infinite loop is expected.

Originally Posted by recklez

3.If i replace the inner for loop with an if statement the program would just end if i enter a number larger than 5.

No, since your outer loop is an infinite loop, it will just loop again to print the menu and read input, without printing "enter the number again:\n". As you probably want to print that "error message", I noted that I would "just write the if-else chain that you wrote inside the inner for loop", i.e., get rid of the inner for loop entirely as in whiteflags' example in post #12.