Best way for looping a menu?

[Codeez]

Hi folks, as a simple project I was doing a (wait for it...) calculator. I use a do-while to function the menu and 'true' to infinite loop the process. User can enter 0 to quit, but an infinite loop seems bad style to me. I'm still learning, but I wonder is there a better way?

int main()
{
	using namespace std;

	enum MenuChoice
	{
		MENU_EXIT,
		MENU_ADD,
		MENU_SUBTRACT,
		MENU_MULTIPLY,
		MENU_DIVISION,
		MENU_MODULO,
		MENU_DEC_TO_BIN,
		MENU_SQUARE_ROOT,
		MENU_FACTOR,
		MENU_FACTORIAL,
		MENU_MEAN_MEDIAN_MODE_RANGE,
		MENU_TIMES_TABLE,
		MENU_FIBONACCI,
		MENU_END // 13
	};

        const string strOperationSuccess = "\nFinished task, Repeating menu:\n";
	const string strMenuDisplay = "What operation would you like to perform?: \n\n"
                                      "1. Addition\n2. Subtraction\n3. Multiplication\n4. Division\n5. Modulo\n\n"
                                      "6. Decimal to Binary\n7. Square Root\n8. Factor\n9. Factorial\n10. Mean, Median, Mode & Range\n"
                                      "11. Times Table\n12. Fibonacci Sequence\n\n0. Exit\n";

	unsigned short unMenuChoice = 0;
	do
	{
		cout << strMenuDisplay << endl;

		// get menu choice
		if (!unMenuChoice)
                    cin >> unMenuChoice; // switch this to function call so we can error process
                    cout << endl;

		// check for valid menu choice then call it
		switch (unMenuChoice)
		{
			case MENU_EXIT:
				cout << "Exiting..." << endl;
				exit(0);
			case MENU_ADD:
				addition();
				cout << strOperationSuccess << endl;
				break;
			case MENU_SUBTRACT:
				subtraction();
				cout << strOperationSuccess << endl;
				break;
			case MENU_MULTIPLY:
				multiplication();
				cout << strOperationSuccess << endl;
				break;
			case MENU_DIVISION:
				division();
				cout << strOperationSuccess << endl;
				break;
			case MENU_MODULO:
				modulo();
				cout << strOperationSuccess << endl;
				break;
			case MENU_SQUARE_ROOT:
				squareRoot();
				cout << strOperationSuccess << endl;
				break;
			case MENU_FACTOR:
				factor();
				cout << strOperationSuccess << endl;
				break;
                        case MENU_FACTORIAL:
                                factorial();
                                cout << strOperationSuccess << endl;
                                break;
			case MENU_MEAN_MEDIAN_MODE_RANGE:
				meanMedianModeRange();
				cout << strOperationSuccess << endl;
				break;
			case MENU_TIMES_TABLE:
				timesTable();
				cout << strOperationSuccess << endl;
				break;
			case MENU_FIBONACCI:
				fibonacci();
				cout << strOperationSuccess << endl;
				break;
			default:
				cout << unMenuChoice << " is not a valid menu choice. Enter again.\n" << endl;
				break;
		}

        // reset
        unMenuChoice = 0;

	} while (true);

	return 0;
}

Anything unrelated to this particular problem, feel free to criticize! :)

[Group of Eight]

I love your organization! In this case I don't think using while(true) is unprofessional. Maybe you can change it to do/while(input!=0). Some people see while(true) loops as bad programming practice, and, although it is true in most cases, I think it is most efficient in your case.

[Codeez]

Thankyou :) The biggest lesson I forced myself the moment I started learning was good formatting.

I figured that might be the case. One day I will upgrade it to a UI one, but as such a simple console app. I guessed it might be ok. I did true though instead of != 0 as you wouldn't make it to the while condition if it was 0, it would just call exit() in the switch.

[andywestken]

Where you have exit(0); I would set a flag to exit the loop, with the do-while loop using this flag.

Edit: actually, there's no need for a flag or the exit call. You can use unMenuChoice to determine whether or not you exit the loop. (You will need to move your reset code to the start of the loop.)

exit(0) should only ever be used in C++ code when things have gone so horribly wrong that you need to bail out immediately.

You should exit the loop and return from main to allow the string destructors to fire and the program's exit routine to run. Edit: I was confusing exit() and abort()...

Andy

PS See respose in this stackoverflow.com thread
return statement vs exit() in main()
http://stackoverflow.com/questions/461449/return-statement-vs-exit-in-main

[andywestken]

Also, how do you know the operation was successful when you return from addition(), subtraction(), etc. What happens if the user enters 1 / 0. ?? Would it be better to return some value to indicate success or failure?

I am also wondering if you could factor out the line

cout << strOperationSuccess << endl;

so it doesn't have to be repeated.

Andy

[Dy7SLyTq]

what if your need to exit from a deeply recursed(<- right word? i think you know what i mean though) function. would exit work better or setjmp? and i think your getting confused with abort, which exits immediatly. exit allows for clean up of objects on single threaded programs. http://www.cplusplus.com/reference/cstdlib/exit/

edit: this was in response to the one two above mine, not directly above

[andywestken]

@DTSCode

Yes, I've confused exit and abort when it comes to the clean-up (edited earlier post.)

The issue here is this (from the reference you gave.)

Note that objects with automatic storage are not destroyed by calling exit (C++).

Which is a big deal in C++

would exit work better or setjmp

If you cannot re-structure the code is a way that avoids the nesting (I think you mean deeply nested?), the least worst solution might well be a goto.

Andy

[Dy7SLyTq]

i mean like this:

void someFunc()
{
     someFunc();
}

i know that they above code is pointless and would never actually happen, but is that called deeply nested?

[andywestken]

Ah, ok.

Obviously I didn't know what you meant before! (And whether goto is a good or bad way to exit loops was on my mind from a recent-ish thread...)

With the recursive case I would only expect early termination to be required under abnormal conditions, in which case exit() makes sense. Under normal conditions I would expect the call sequence to be left to complete.

(I am assuming the code fragment was intended to terminate, even though there's no explicit condition?)

Andy

[Codeez]

I have it setup so the switch calls a void function, when that finishes (the maths task) the switch is finished and we loop again. There is nothing returned in those functions. E.g. squareRoot():

void squareRoot() // TODO: input error checking
{
	using namespace std;

	cout << " == Square Root ==\n" << endl;
	cout << "Enter a number to square root: ";

	double dInputSqRt;
	cin >> dInputSqRt;

	// if positive, workout then print
	if (dInputSqRt >= 0)
		cout << "The square root of " << dInputSqRt << " is " << sqrt(dInputSqRt) << endl;
	// if negative (imaginary), workout then print
	else if (dInputSqRt <= -1)
	{
		// convert it to a positive then append an 'i'
		dInputSqRt *= -1;
		cout << "The square root of " << dInputSqRt << " is " << sqrt(dInputSqRt) << "i" << endl;
	}
}