Console menu, input stream - keeping clean code

Question

I've written simple numeric menu which displays in the console. When user clicks '1', something happens, then when he clicks enter, I clear the whole output except for the menu itself. When the user clicks '5', program ends.

However, the code got ugly. It's full of cin.get()'s, cout's, and other crap like that.

Let me post it here:

int main()
{
    using namespace std;
    node *root = NULL;
    char ch = '0';

    while(ch != '5')
    {
        cout << "1. Add new elements\n"
            "2. Display info about the tree\n"
            "3. Remove nodes\n"
            "4. Export to file\n"
            "5. Exit\n";
        cin >> ch; cin.get();

        switch(ch)
        {
            case '1': case '3':
            {
                vector<int> numbers;
                string line;
                cout << "\nProvide numbers divided with spaces: ";
                getline(cin, line);
                istringstream in(line, istringstream::in);
                int current;
                while (in >> current) numbers.push_back(current);

                (ch == '1') ? insert(root, numbers) : remove(root, numbers);
                cout << "\nClick to return to menu";
                cin.get();
                break;
            }
            case '2':
            {
                displayTreeInfo(root, cout);
                cout << "\n\nClick to return to menu";
                cin.get();
                break;
            }
            case '4':
            {
                string name;
                cout << "Provide filename: ";
                cin >> name;

                buildTree(root, loadNumbersFromFile(name));
                exportTreeToFile(root, name);
                std::cout << "\nData has been exported.\n\n";
                cin.get(); cin.get();
                break;
            }
        }
        system("cls");
    }

    exit(EXIT_SUCCESS);
}

How could I improve this code?

Have you compiled this code yourself before? I'm doing it now, and it already says that node is undeclared (obvious when just looking at it, too). Also, why is using namespace std inside of main?
Okay, assuming this is not the entire program (though it would be nice to see all of it), I'll see what can be cleaned here.
This is only part of the code. Rest of it, structure (node), functions etc. looks o.k. for me, so I didn't post it. According to the using namespace std inside of main - I've heard it's a good practice to place it there instead of at the top of the program. Isn't it true?
Thank you, I'm waiting for your suggestions. If you want to see the whole code, I can post it on ideone, here - ideone.com/gCcGlv. It compiles and works in VS 2012, though ideone shows a couple of errors (maybe I chose wrong c++ standard from the list, I don't know).
I haven't heard that before, but I do know that it itself isn't good practice. Read this post for more information: stackoverflow.com/questions/1452721/…. Beyond that, thanks for the clarification. I'll proceed, but I'll ignore the node things in your code.

Jamal · Accepted Answer · 2013-04-20 22:27:49Z

Using namespace std: I've already mentioned this in the comments. Although it's not a huge deal for small programs such as this, it's still not a good habit to develop.
While loop: Put std::cin << ch both before the loop and after the switch. You could also change the loop condition to

(ch == '1' || ch == '2' || ch == '3' || ch == '4')

if you really want to make sure that the program will only proceed with a valid input. Other than that, your original condition is okay.
std::cin: You don't need std::cin.get() alongside std::cin >>. Just use std::cin >> for char inputs. However, you'll need a std::cin.ignore() before each getline since you're doing a char input (menu prompt) before a std::string input in your cases. For case 4, keep a consistent getline for the filename input and put an ignore before it. Also, you don't need any cins after your cases since your while loop will always ask for an input for ch. I do see what you're doing with your outputs in your cases, but you don't need to do that. Just let your cases do their things and go straight to the menu and/or prompt again.
Filename input: You should provide validation for the filename input. If the user enters in invalid filename, what should happen? Consider implementing a loop asking the user for a valid filename as long as such file cannot be opened. With the provided code here, it's a bit tough for me to give a piece of example code for that.
Functions: You could nicely organize your switch statements by putting the body code into functions. Just name them as they appear in your menu. Call each one in your respective cases, along with the breaks. Also, it might be better to have separate procedures for add/remove.
Exit: You don't really need exit(EXIT_SUCCESS) here if the program will always terminate successfully. You also don't need system("cls"), so just leave it out. If you're doing that just to prevent the menu from appearing before each prompt, just pt it before the while loop.

You're welcome. I did leave out something, so I'll add it in. If you're still curious about the filename input validation, you can always request an idea on Stack Overflow.

asked	2 months ago
viewed	57 times
active	2 months ago

Console menu, input stream - keeping clean code

1 Answer

Your Answer

Not the answer you're looking for? Browse other questions tagged c++ or ask your own question.

Console menu, input stream - keeping clean code

1 Answer

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged c++ or ask your own question.

Related