Looping void function

[tintin123]

void menu()is not looping as intended. How do I fix this?

#include <iostream>
using namespace std;
void menu()
{
	int apples=0, oranges=0, cherries=0, watermelons=0, total, menu1, balance=100, fruits;
	do
	
	{
		cout << "\n\n Welcome to BETLOG'S\n\n";
		cout << "Menu:\n\n";
		cout << "(1) Apples		$1\n";
		cout << "(2) Oranges		$2\n";
		cout << "(3) Cherries		$3\n";
		cout << "(4) Watermelons		$4\n";
		cout << "You have:\n" << apples << " Apples \n"<< oranges <<" Oranges \n" << cherries << " Cherries \n" << watermelons << " Watermelons \n";
		cout << "Available balance is $" << balance << ".\n\n";
		cout << "Type the number that represents the action you want.";
		cin >> fruits;
	switch (fruits) 
	{
		case 1:
			cout<<"How many apples would you like?";
			cin>>fruits;
			apples +=1*fruits;
			total = total- (1*fruits);
			break;
		case 2:
			cout<<"How many oranges would you like?";
			cin>>fruits;
			oranges +=1*fruits;
			total = total- (2*fruits);
			break;
		case 3:
			cout<<"How many cherries would you like?";
			cin>>fruits;
			cherries +=1*fruits;
			total = total- (3*fruits);
			break;
		case 4:
			cout<<"How many watermelons would you like?";
			cin>>fruits;
			watermelons +=1*fruits;
			total = total- (4*fruits);
			break;
		}
	}
while (menu1==0);
}
int main()
{

int userchoice,menu1;
int apples=0, oranges=0, cherries=0, watermelons=0, total, menu2, balance=100, fruits;
    cout<<"______________________________________________________________________\n";
    cout<<"|                                                                    |\n",
    cout<<"| ##      ## ######## ##        ######   #######  ##     ## ######## |\n";
    cout<<"| ##  ##  ## ##       ##       ##    ## ##     ## ###   ### ##       |\n";
    cout<<"| ##  ##  ## ##       ##       ##       ##     ## #### #### ##       |\n";
    cout<<"| ##  ##  ## ######   ##       ##       ##     ## ## ### ## ######   |\n";
    cout<<"| ##  ##  ## ##       ##       ##       ##     ## ##     ## ##       |\n";
    cout<<"| ##  ##  ## ##       ##       ##    ## ##     ## ##     ## ##       |\n";
    cout<<"| ###  ###  ######## ########  ######   #######  ##     ## ########  |\n";
    cout<<"|____________________________________________________________________|\n";

do{
cout<<"***********************************************************\n\n\n[1]Menu\n[2]Cart\n[3]Pay-up"<<endl;
cin >>userchoice;
	switch (userchoice)
	{
		case 1:
			menu2+1;
			menu();
			break;
		case 2:
			return 0;
			break;
		default :
			cout<<"Invalid Input";
	}
}
while (userchoice!=2);
}

[seeplus]

while (menu1==0);

The initial value of menu1 is not defined and it's value isn't changed in the do loop.

[Shan Vikram]

menu2 is also not initialized but you are using in line no 71 with out initial value. Always try to initialize proper value for local variables or else it is initialized with garbage value and result in undefined behavior.

Duplication of "menu1" variable in main function as well . If not used then delete it, this unused variables also occupies memory, especially in embedded products given memory space is limited and it is developers responsibility use the available memory effectively.

[Handy Andy]

Hello tintin123,

Been working on this most of the morning. I have not fixed everything as the comments will show, but should point you in the right direction.

Sorry I had to break this up into 2 posts.

int main()
{
    bool notDone{ true };  // <--- Added.
    int menuChoice{}/*, menu1*/;
    int apples{}, oranges{}, cherries{}, watermelons{}, total{}, menu2{}, balance{ 100 }, fruits{};  // <--- None of these are used in "main". Is the a need for them?

    std::cout <<
        "___________________________________________________________________________\n"
        "|                                                                          |\n"  // <--- Originally ended with a (,).
        "| ##      ##  #######   ##         ######    #######   ##     ##  #######  |\n"
        "| ##  ##  ##  ##        ##        ##    ##  ##     ##  ###   ###  ##       |\n"
        "| ##  ##  ##  ##        ##        ##        ##     ##  #### ####  ##       |\n"
        "| ##  ##  ##  ######    ##        ##        ##     ##  ## ### ##  ######   |\n"
        "| ##  ##  ##  ##        ##        ##        ##     ##  ##     ##  ##       |\n"
        "| ##  ##  ##  ##        ##        ##    ##  ##     ##  ##     ##  ##       |\n"
        "|  ###  ###   ########  ########   ######    #######   ##     ##  ######## |\n"
        "|__________________________________________________________________________|\n";

    std::cout << "\n" << std::string(76, '*') << "\n\n";

    do
    {
        //std::cout << "***********************************************************\n\n\n[1]Menu\n[2]Cart\n[3]Pay-up" << endl;
        std::cout << 
            "[1]Menu\n"
            "[2]Cart\n"
            "[3]Pay-up\n"
            " Enter choice: ";  // <--- Needs an "Exit" choice.
        std::cin >> menuChoice;

        // <--- Need to check if input is NOT a number.

        switch (menuChoice)
        {
            case 1:
                //menu2 + 1;  // <--- Never used. And what is your point for this?

                menu();

                //menuChoice = 0;

                break;
            case 2:
                return 0;
                break;  // <--- This line never reached.
            case 4:
                notDone = false;

                break;
            default:
                std::cout << "\n     Invalid Input!\n\n";
        }
    } //while (menuChoice < 1 || menuChoice > 3);  // <--- while (notDone);
    while (notDone);

    return 0;  // <--- Not required, but makes a good break point.
}

Line is a way to make this easier to work with. Since all lines are strings the IDE and compiler considers this a 1 large string of 770 characters. Therefor you only need 1 "cout" and ";" for everything. The advantage is that it looks more like what you will eventually see on the screen. Also it makes it easier to make changes like the last line when I added a space at the beginning to make it line up better with the lines above. I also added a space between the letters and took 1 (#) off the first line of the (E). Changing the first line of the (E) is not necessary, but I thought it looked better. Your choice.

Line 19 is the same as the beginning of line 23. It makes use of the fill ctor of the string class. I find it easier to work with and adjust. Use whichever 1 works best for you.

Line 24 is the same as the first "cout", but when you add a choice for "Exit" you will see how easy it is to work with.

For line 31. I realize that you are probably in school where no one does anything wrong and nothing ever will go wrong with your program, but eventually you will learn to check for errors and other problems, so now is a good time to start. What I mean here is what is someone enters a letter or other non numeric character? If this happens, because of the formatted input std::cin >> menuChoice;. "cin" will become unusable the rest of the program and will not stop for any new input from the keyboard. You need to check for this and fix it before the program continues or it will not work. You could do this with a while loop.

In "cast 1", line 40, I started with this, but decided this is not the best way to accomplish what is needed. That is why I added the bool variable "notDone". The same concept can be used in the "menu" function.

In regards to your function or what should be functions.

A function should do 1 thing and return.

Then there is the name "menu". What kind of menu? The name is misleading because you do not know what kind of menu it is and it does more than just print a menu to the screen.

If you want to keep the function as is that is fine, but give it a name that better describes what it is.

The next question is that you define variables used in this function, but are they need somewhere else? Should they be passed to other functions or defined in "main" where they can be passed to the functions that need them?

End part 1.

[Handy Andy]

Part 2:

Given your original code I have worked this up, but it could be better, so use it as a guide and not a solution.

void menu()
{
    constexpr int APPLES{ 1 }, ORANGES{ 2 }, CHERRIES{ 3 }, WATERMELLONS{ 4 };

    int apples{}, oranges{}, cherries{}, watermelons{}, total{}, menuChoice{}, balance{ 100 }, fruits{};
    
    do
    {
        std::cout <<
            "\n\n Welcome to BETLOG'S\n\n"
            "Menu:\n\n"
            "(1) Apples         $1\n"
            "(2) Oranges        $2\n"
            "(3) Cherries       $3\n"
            "(4) Watermelons    $4\n"
            "You have:\n" << apples << " Apples \n" << oranges << " Oranges \n"
            << cherries << " Cherries \n" << watermelons << " Watermelons \n"
            << "Available balance is $" << balance << ".00.\n\n"  // <--- Changed.
            << "Type the number that represents the action you want. ";
        
        while (!(std::cin >> menuChoice))
        {
            // <--- Reser the state bits.
            // <--- Clear the input buffer.
        }

        switch (menuChoice)
        {
            case 1:
                std::cout << "How many apples would you like? ";
                std::cin >> fruits;

                apples += fruits;

                total = total - (APPLES * fruits);  // <--- Do you mean "balance -= APPLES * fruits"?

                menuChoice = 0;
                break;
            case 2:
                std::cout << "How many oranges would you like? ";
                std::cin >> fruits;

                oranges += fruits;

                total = total - (ORANGES * fruits);
                menuChoice = 0;

                break;
            case 3:
                std::cout << "How many cherries would you like? ";
                std::cin >> fruits;

                cherries += 1 * fruits;

                total = total - (CHERRIES * fruits);

                menuChoice = 0;
                break;
            case 4:
                std::cout << "How many watermelons would you like? ";
                std::cin >> fruits;

                watermelons += 1 * fruits;

                total = total - (WATERMELLONS * fruits);

                menuChoice = 0;
                break;
        }
    } while (!menuChoice);
}

Starting at line 12 I can only guess that you used the "tab" key space out what you have. This may have worked for now, but beyond the first character of a string the "tab" key or "\t" may not always space correctly. It is better to just use spaces to achieve what you want. It also makes your code easier to understand and work with.

In this example of the "cout" you can see how the insertion operator, (<<), is used to chain the strings and variables together.

The while loop here is an example of what I said you need in "main". Since I have not finished the code yet it is untested at the moment.

I have only worked with "case 1" as far as running and testing. That is why line 35 is the only one with a comment.

I think you are misusing "total" and not using "balance" as you should be. This becomes apparent when you loop and still have the same staring balance for your output. You have the variable "total", but other than adding to the garbage value of the variable you never use it for anything. Do you have a use for this variable?

Again like "main" you need a 5th choice for "Exit" and a case statement to match.
This would allow you to use the same concept as I did in "main" and get rid of the lines menuChoice = 0;. Having to use this in every case means there is a better way to do this.

Between seeplus and Shan Vikram I would add that it is ALWAYS a good practice to initialize your variables when defined. Especially variables like "total" or something similar. You would not want a variable like "total" to have a starting value of "-858993460", usually what I get on my computer and also called a garbage value, and then add to this only to find the the end value is a number that is a little closer to (0) zero.

Andy