0

I'm a bit stumped at what I'm missing in my Greedy solution. The program keeps getting hung up right after the prompt for user input. Check50 doesn't give me much information to work from. What is causing the program to hang?

#include <stdio.h>
#include <cs50.h>
#include <math.h>

int main(void)
{
   int input;
   int quarter, dime, nickel, penny;
   int remaining;

do
{
   printf("O hai! How much change is owed?");
   input = GetFloat();
}

while (input <= 0);

int cents = round(input * 100);
int coins = 0;
int counter1, counter2, counter3, counter4;

while (cents >= 25)
{
    quarter = (cents/25);
    counter1 = quarter;
    remaining = (cents % quarter);
}

while (cents >= 10)
{
    dime = (remaining/10);
    counter2 = dime;
    remaining = (remaining % 10);
}

while (cents >= 5)
{
    nickel = (remaining/5);
    counter3 = nickel;
    remaining = (remaining % 5);
}

while (cents >=1)
{
    penny = (remaining/1);
    counter4 = penny;
    remaining = (remaining % 1 );
}

coins = (counter1 + counter2 + counter3 + counter4);

printf("%d\n", coins);
}

I tried a different approach and am not getting any errors when I run check50 anymore. But, it still hangs after the prompt. Here is my different approach:

#include <stdio.h>
#include <cs50.h>
#include <math.h>

int main(void)
{
    int input;
    int coins = 0;
    int counter1 = 0;
    int counter2 = 0;
    int counter3 =0;
    int counter4 = 0;
    int quarters, dimes, nickels, pennies;
    int remaining = 0;


     printf("O hai! How much change is owed?");
     input = GetFloat();
     int cents = round(input * 100);

   for ( ; cents >= 25; cents--)
    {
       quarters = (cents/25);
       counter1 = quarters;
       remaining = (cents % quarters);
    }

  for ( ;remaining >= 10 || remaining <= 24; remaining--)
    {
       dimes = (remaining/10);
       counter2 = dimes;
       remaining = (remaining % 10);
    }

for ( ; remaining >= 5 || remaining <= 9; remaining--)
    {
       nickels = (remaining/5);
       counter3 = nickels;
       remaining = (remaining % 5);
    }

 for ( ; remaining >=1 || remaining <= 4; remaining--)
    {
       pennies = (remaining/1);
       counter4 = pennies;
       remaining = (remaining % 1 );
    }

 coins = (counter1 + counter2 + counter3 + counter4);

 printf("%d.2\n", coins);

    }
1
  • 1
    Please show us what check50 says – lethaljd Nov 25 '14 at 13:05
3

I keep getting hung up right after the prompt for user input.

This is a clear indication that there is an infinite loop (at least once). The problem is that you have 4 infinite loops, the four while loops. You write that

while (cents >= some_constant)
{
      //your loop's inner stuff
}

But you don't change the value of cents, so this condition always yields true, hence the first most while loop gets into an infinite iteration. Either try removing this infinite looping, but as far as I can predict on basis of your code, you wished to use an if statement instead of while. But still your solution can be optimized further in terms of memory.

Any ways, Good Luck.

0

I'm going with sinister. You don't want to use while loops. Also defining Quarter, Nickel and Dime as Constants might help a bit more too. Pennies you don't have to worry about since a penny equals 1 cent which means you're dividing an amount by 1 which equals that amount ie 10 / 1 = 10, which is redundant code. Hope this helps.

0

In your second attempt, using for loops, you have several conditions that will never be evaluated to be false.

Take the last one, for example:

remaining >=1 || remaining <= 4

There are three cases to consider with respect to the value of remaining. It could be:

  • Greater than 4
  • Less than 1
  • Between 1 and 4 (inclusive)

Any value for a particular case will behave the same way as all other values for that same case; in other words, it makes no difference whether the value of remaining is 5 or 5,000 because both of those values are in the "greater than 4" case. So, choose an easy value to work with that belongs to each of the three cases. I'll choose 10 for the first case, 0 for the second, and 2 for the third, with the goal of evaluating the condition for each case to see when it will be true and when it will be false.

First case (10): The expression says, "10 is greater than or equal to 1 or 10 is less than or equal to 4." Since you used the "or" operator, the expression is true if either part is true. The second part is false, but the first part is true, so this case is true.

Second case (0): The expression says, "0 is greater than or equal to 1 or 0 is less than or equal to 4." Again, we only need one or the other to be true for the whole expression to be true, and the second part is true – 0 is less than 4 – so this case is also true.

Third case (2): The expression says, "2 is greater than or equal to 1 or 2 is less than or equal to 4." In this case, both sides of the expression are true, so this case is clearly true.

Those are the only possibilities to consider, and they all evaluate as true. Any other number will fall into one of the same three cases, with the same result. That's why your second attempt also produced an infinite loop.

The most direct way to fix this would be to change || to && so that both sides have to evaluate as true for the whole expression to be true. Then, only the third case where your value is between the numbers you give as endpoints will continue the loop; the other cases, where remaining is outside the range you specify, will cause the loop to end. However, don't be surprised when your code has more problems after fixing the infinite loop. Ask yourself: Do you really want to do what's in the body of each loop multiple times?

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .