Solving the "Pancake Glutton" problem.

[AILD312]

I did my best to try and solve this problem so i thought maybe some more experienced people would have some tips to simplify and or improve my solution to it:

#include <iostream>
using namespace std;


int main()
{
    int x;
    int z;
    int MaxEaten;
    int MinEaten;
    int MaxPers = 1;
    int MinPers;
    int PancakesEaten[3];

    cout << "How many pancakes did each person eat?" << endl;

    for (x = 0; x < 3; x++)
    {
        cout << "Person " << x + 1; cout << ": ";
        cin >> PancakesEaten[x];
    }

    MaxEaten = PancakesEaten[0];

    for (z = 0; z < 3; z++)
    {
        if (MaxEaten < PancakesEaten[z])
        {
            MaxEaten = PancakesEaten[z];
            MaxPers = z + 1;
        }

        if (MinEaten > PancakesEaten[z])
        {
            MinEaten = PancakesEaten[z];
            MinPers = z + 1;
        }
    }

    cout << "\nPerson " << MaxPers << " ate the most pancakes:  " << MaxEaten << endl;
    cout << "\nPerson " << MinPers << " ate the least pancakes: " << MinEaten << endl;
}

[Ben Duncan]

What is the Pancake Glutton problem?
What are you having problems with?

You'll have to answer these before I can try to help.

[AILD312]

Sorry didn't think about it, here's the problem:

Write a program that asks the user to enter the number of pancakes eaten for breakfast by 10 different people (Person 1, Person 2, ..., Person 10) Once the data has been entered the program must analyze the data and output which person ate the most pancakes for breakfast.

Only 1 place confuses me:
int MaxPers = 1; - if i don't set this variable value to 1, whenever the person with the max number is person "1", instead of "1" it prints out "2686760".

This is the only thing i'm not sure about. Why do i get such an error only with "MaxPers", when the person who ate the least is person "1" the program runs perfectly, "MinPers" not being given any value from the start.

The thing i was hoping for when creating this topic was to get some advices from more experienced people, like... is my code readable enough, can it be simplified or what other ways are there to solve such a problem and etc.

[Stupebrett]

cout << "Person " << x + 1; cout << ": ";

can be changed to

cout << "Person " << x + 1 << ": ";

Other than that the program seems fine. I can't really say if it is a better way to solve it, since I haven't done the problem myself.

[Dacster13]

the second for loop isn't really necessary in my opinion.

#include <iostream>
using namespace std;

const int ARRAY_SIZE = 3;

int main()
{
    int MaxEaten                    = 0;
    int MinEaten                    = 0;
    int MaxPers                     = 1;
    int MinPers                     = 1;
    int PancakesEaten[ARRAY_SIZE]   = {0};

    cout << "How many pancakes did each person eat?" << endl;

    for (int x = 0; x < ARRAY_SIZE; x++)
    {
        cout << "Person " << x + 1; cout << ": ";
        cin >> PancakesEaten[x];
        
        // check if this is the first iteration of the loop
        if(x == 0)
        {
             // set the initial values we need, for our conditional checking later
             MaxEaten = PancakesEaten[0];
             MinEaten = PancakesEaten[0]; 
        }// end if
        
        if (MaxEaten < PancakesEaten[x])
        {
            MaxEaten = PancakesEaten[x];
            MaxPers = x + 1;
        }// end if

        if (MinEaten > PancakesEaten[x])
        {
            MinEaten = PancakesEaten[x];
            MinPers = x + 1;
        }// end if
    }// end for

    cout << "\nPerson " << MaxPers << " ate the most pancakes:  " << MaxEaten << endl;
    cout << "\nPerson " << MinPers << " ate the least pancakes: " << MinEaten << endl;
    return 0;
}// end of main 

That's what comes to me at the moment. Not sure if there's an even better way of doing it though.

Only 1 place confuses me:
int MaxPers = 1; - if i don't set this variable value to 1, whenever the person with the max number is person "1", instead of "1" it prints out "2686760".

That's because you didn't initialize the MaxPerson variable to any value so it has some garbage value, for the MinPerson it's ok cause when you get to this point in the code.

if (MinEaten > PancakesEaten[z])

MinPerson also has some random garbage value, but since the garbage value is large MinEaten > PancakesEaten becomes true and your MinEaten value gets set to the value that PancakeEaten[z] has. Whereas for the MaxEaten variable the condition

if(MaxEaten < PancakesEaten[z])

will never become true, because MaxEaten has a garbage value that is higher than what you entered into PancakesEaten[z]. So it's value never gets changed and it is printed out in the end. Also even if it works for the MinPerson, it isn't guaranteed to work all the time because you'll never be sure what the garbage value would be.

That's the reason why it's good programming practice to initialize your variables when you declare them. So you'll always know what your variables hold.

[AILD312]

will never become true, because MaxEaten has a garbage value that is higher than what you entered into PancakesEaten[z]. So it's value never gets changed and it is printed out in the end. Also even if it works for the MinPerson, it isn't guaranteed to work all the time because you'll never be sure what the garbage value would be..

Thanks A LOT for this one, it really didn't cross my mind to check it step by step how the program works even though i knew about the garbage values assigned to an uninitialized variable.

[Dacster13]

You're welcome, glad I could help. =)

[qczackf]

Mmmkay... simple enough.

Lets begin with your second for loop. I see you iterate over each variable within the gathered user input and each loop iteration represents a single person, with a total of three iterations, or three people. And after determining if the value is less/greater-than the current, you assign the new value to the Min/Max variable:

    for (z = 0; z < 3; z++)
    {
        if (MaxEaten < PancakesEaten[z])
        {
            MaxEaten = PancakesEaten[z];
            MaxPers = z + 1;
        }

        if (MinEaten > PancakesEaten[z])
        {
            MinEaten = PancakesEaten[z];
            MinPers = z + 1;
        }
    }

When you are programming (in any language) you need to look for patterns that can 'tighten' your code. Not neccessarily make it smaller, or simpler, but to improve the readability so that your code can be easily understood, not read, by other developers. Being a small piece, your code is understood easily.

Now, lets look at the first 'for' loop (with all my comments):

    int x; // hmmm, used as iterator
    int z; // hmmm, aswell

    int MaxEaten;  // maximum pancakes eaten
    int MinEaten;  // minimum pancakes eaten
    int MaxPers = 1;  // the person who ate the most
    int MinPers;      // the person who ate the least
    int PancakesEaten[3]; // hmmm, store min and max variables

    // well written sentence
    cout << "How many pancakes did each person eat?" << endl;

    // hmmm, constant three iterations 
    for (x = 0; x < 3; x++)
    {
        cout << "Person " << x + 1; cout << ": ";
        cin >> PancakesEaten[x];
    }

I see you are using the x and z variables as incrementers in the for loop and only incrementing them three times (once per iteration) with each iteration representing a single person.

Do you see any patterns? Instead having the the two incrementers, you could provide constant that represents the maximum number of loops to peform, or number of persons eating pancakes:

    const int MaxPeople = 3; // 'const' modifier tells the compiler the value never changes 

In for loops, you are allowed to declare temporary incrementers and using the above constant, your for loops can be written as:

    for(int i = 1; i <= MaxPeople + 1; i++) // 'i = 1' and check for '<= MaxPeople + 1'
    {
        // ...
    }

Additionally you can change:

    int PancakesEaten[3]; // to:
    int PancakesEaten[MaxPeople];

You programs behavior can now be influenced by the MaxPeople constant, verses hand written constants, like '3'.

The next part depends on whether or not, your professor, wants the data to be 'stored' and later operated on, or if your application can process data as its recieved.

If he wants the data to be stored and later operated on, your assignment can only contain the modified for loops by applying the above syntax and replacing x and z with i:

    for(int i = 1; i <= MaxPeople + 1; i++)
    {
        // cout << "Person " << i; cout << ": "; 
        // can be cleanly written as:

        cout << "Person " << i << ": "; // all << (goes into) cout

        // the cout '<<' operator can be 'chained'

        cin >> PancakesEaten[i];
    }

    // ...
  
    for(int i = 1; i <= MaxPeople + 1; i++)
    {
        if (MaxEaten < PancakesEaten[i])
        {
            MaxEaten = PancakesEaten[i];
            MaxPers = i + 1;
        }

        if (MinEaten > PancakesEaten[i])
        {
            MinEaten = PancakesEaten[i];
            MinPers = i + 1;
        }
    }

For the sake of learning, we wil further optimize this code (for readability).

Instead of using the PancakesEaten[3] array, you can ditch the array completely and process user input data as its received. First, move the contents of the second for loop, into the first for loop, directly after cin << PancakesEaten[i] and introduce a loop local variable used to store user input, rewritten as:

    // ...

    for(int i = 1; i <= MaxPeople + 1; i++)
    {
        cout << "Person " << i << ": "; // all << (goes into) cout

        // and change
        // cin >> PancakesEaten[i];
        // to:

        int userinput; 
        cin >> userinput;

	// a zero will create strange clauses below, introducing a bug 
        // so lets disqualify the persons answer, decrement i, and jump 
        // to the start of the loop
	if (userinput == 0)
	{
	   cout << "Impossible!" << endl; // endl is within the std:: namespace, represents end of line char
	   
           i--; //  and lets not skip the person

	   // keyword, meaning: skip into the next iteration of the loop that
           // current code scope belong to (including 'while' and 'do-while' loop)
	   // ignoring everything below
           continue;
	}

	if (MaxEaten == 0)
	{
	    MaxEaten = userinput; // single line if statement doesn't need {}
	}
        else if (userinput > MaxEaten) // note: checking userinput, not MaxEaten
        {
            MaxEaten = userinput;
            MaxPers = i;
	}

        if MinEaten == 0)
	{
	    MinEaten = userinput; // single line if statement doesn't need {}
	}
        else if (userinput < MinEaten) // note: checking userinput, not MinEaten
        {
            MinEaten = userinput;
            MinPers = i;
        }
    } 

   // ... 

To ensure that you don't get confusing results from your variables, assign a default value to them, and rewritten, the new approach looks like:

    const int MaxPeople = 3; // more flexible! modifies the loop!

    int MaxEaten = 0; // not '1', nobody ate pancakes yet!
    int MinEaten = 0; // ...like I said
    int MaxPerson = 0;
    int MinPerson = 0; 

    cout << "How many pancakes did each person eat?" << endl;

    for(int i = 1; i <= MaxPeople + 1; i++)
    {
	cout << "Person " << i << ": "; // all << (goes into) cout

	int userinput; 
	cin >> userinput; 

	if (userinput == 0)
	{
  	    cout << "Impossible! Try that again!" << endl; 
	    i--; 
	    continue;
	}

	if (MaxEaten == 0)
	    MaxEaten = userinput; 
	else if (userinput > MaxEaten) 
	{
	    MaxEaten = userinput;
	    MaxPers = i;
	}

	if MinEaten == 0)
	    MinEaten = userinput;
	else if (userinput < MinEaten)
	{
	    MinEaten = userinput;
	    MinPers = i;
    	}
    }

    // 'endl' == '\n', usually
    cout << "Person " << MaxPerson << " ate the most pancakes:  " << MaxEaten << endl;
    cout << "Person " << MinPerson << " ate the least pancakes: " << MinEaten << endl;

    // always return '0', because the OS would like to know how the application 'exited'
    // also known as the 'exit code'
    return 0;

I hope this helps you with finding patterns, so that you can write better code. As far as the actual confusion you have about the value of MaxEaten, it could be a couple things, but don't quote me.

Most compiler will default values of most built-in data types to zero or false, or infinity. Sometimes, when receiving really strange results, the problem may not be in your written code or be an issue of software and the problem could exist as a hardware issue, possibly in your computers RAM. Your application is loaded into RAM when executed and if a variable is stored within an area of RAM that contains 'bad bits', the CPU will receive false-positive bits, resulting in a dramatic change within the value of the variable any 'bad bits' belong to.

When it happens again, try looking into using a debugger, so you can step through each line of code as it executes and watch the values of variables.

Research, research, research, practice.

Happy holidays!

EDIT: I need to type faster.