Infinite loop although I put a break;

Question

The aim of this algorithm is to return an organized array (with currencies) of the change .

I built a nested while loop to loop as long as "change" is not equal 0 then each loop in the while loop function while "change" is more than 0 (i.e : not -ve).

What have I missed?

// 
function checkCashRegister(price, cash, cid) {
  var change = cash-price;
  var chArr = [["PENNY", 0],
["NICKEL", 0],
["DIME", 0],
["QUARTER", 0],
["ONE", 0],
["FIVE", 0],
["TEN", 0],
["TWENTY", 0],
["ONE HUNDRED", 0]];

  while (change !== 0) {
    while (change - 100 > 0) {
      chArr[8][1] += 100;
      change -=100;
      if (change <= 0) {break;}
    }
    while (change - 20 > 0) {
      chArr[7][1] += 20;
      change -=20;
      if (change <= 0) {break;}
    }
    while (change - 10 > 0) {
      chArr[6][1] += 10;
      change -=10;
      if (change <= 0) {break;}
    }
    while (change - 5 > 0) {
      chArr[5][1] += 5;
      change -=5;
      if (change <= 0) {break;}
    }
    while (change - 1 > 0) {
      chArr[4][1] += 1;
      change -=1;
      if (change <= 0) {break;}
    }
    while (change - 0.25 > 0) {
      chArr[3][1] += 0.25;
      change -=0.25;
      if (change <= 0) {break;}
    }
    while (change - 0.1 > 0) {
      chArr[2][1] += 0.1;
      change -=0.1;
      if (change <= 0) {break;}
    }
    while (change - 0.05 > 0) {
      chArr[1][1] += 0.05;
      change -=0.05;
      if (change <= 0) {break;}
    }
    while (change - 0.01 > 0) {
      chArr[0][1] += 0.01;
      change -=0.01;
      if (change <= 0) {break;}
    }
    if (change <= 0) {break;}

  }
  // Here is your change, ma'am.
  return chArr;
}

checkCashRegister(17.46, 20.00, [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.10], ["QUARTER", 4.25], ["ONE", 90.00], ["FIVE", 55.00], ["TEN", 20.00], ["TWENTY", 60.00], ["ONE HUNDRED", 100.00]]);

@Thomas There's a break in the outer loop too. But yeah it's overly complex — Hespen, Nov 17 '16 at 09:47
Try to go through it step by step with a browser's developer tools? — RemcoGerlich, Nov 17 '16 at 09:47
there's no need to break though i put it as a final attempt to fix things @Thomas @A — BahaaZidan, Nov 17 '16 at 09:47
@BahaaZidan Please use modulo... This is a first step to simplify your code. Seriously. — Amessihel, Nov 17 '16 at 09:48
@BahaaZidan: way too much duplicated code. You should change the outer while to `while (change > 0)` anyway, checking that it's unequal to 0 makes you vulnerable to float rounding errors. But the final break deals with that, so I don't see the bug. — RemcoGerlich, Nov 17 '16 at 09:49
http://stackoverflow.com/questions/2664301/how-does-modulus-divison-work — mplungjan, Nov 17 '16 at 09:49
Actually, remove the outer loop. If the inner loops work, the end result should always be that you've made change, period. — RemcoGerlich, Nov 17 '16 at 09:50
Also why not use the names once : If you do `{ "hundred":0,...}` you could do `chArr["hundred"] += 100;` — mplungjan, Nov 17 '16 at 09:51
@RemcoGerlich thans aloooot m8 (y) works like magic now .. i really appreciate it — BahaaZidan, Nov 17 '16 at 09:53
@BahaaZidan: check my answer; if you only removed the outer loop, your answers are all going to be wrong by 1 cent (and things will be paid out with the wrong coins). — RemcoGerlich, Nov 17 '16 at 09:59

Nina Scholz · Accepted Answer · 2016-11-17T10:38:25.237

You need just to check the single cash parts and use there only the check if the rest is greater then the change. No need for a breaking condition inside of the while loop.

What is missing, is to check if the change is sufficient.

function checkCashRegister(price, cash, cid) {
  var change = cash-price;
  var chArr = [["PENNY", 0], ["NICKEL", 0], ["DIME", 0], ["QUARTER", 0], ["ONE", 0], ["FIVE", 0], ["TEN", 0], ["TWENTY", 0], ["ONE HUNDRED", 0]];

    while (change >= 100 && cid[8][1] >= 100) {
      chArr[8][1] += 100;
      cid[8][1] -= 100;
      change -=100;
    }
    while (change >= 20 && cid[7][1] >= 20) {
      chArr[7][1] += 20;
      cid[7][1] -= 20;
      change -=20;
    }
    while (change >= 10 && cid[6][1] >= 10) {
      chArr[6][1] += 10;
      cid[6][1] -= 10;
      change -=10;
    }
    while (change >= 5 && cid[5][1] >= 5) {
      chArr[5][1] += 5;
      cid[5][1] -= 5;
      change -=5;
    }
    while (change >= 1 && cid[4][1] >= 1) {
      chArr[4][1] += 1;
      cid[4][1] -= 1;
      change -=1;
    }
    while (change >= 0.25 && cid[3][1] >= 0.25) {
      chArr[3][1] += 0.25;
      cid[3][1] -= 0.25;
      change -=0.25;
    }
    while (change >= 0.1 && cid[2][1] >= 0.1) {
      chArr[2][1] += 0.1;
      cid[2][1] -= 0.1;
      change -=0.1;
    }
    while (change >= 0.05 && cid[1][1] >= 0.05) {
      chArr[1][1] += 0.05;
      cid[1][1] -= 0.05;
      change -=0.05;
    }
    while (change > 0 && cid[0][1] >= 0.01) {
      chArr[0][1] += 0.01;
      cid[0][1] -= 0.01;
      change -=0.01;
    }
  
  // Here is your change, ma'am.
  return chArr;
}

var register1 = [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.10], ["QUARTER", 4.25], ["ONE", 90.00], ["FIVE", 55.00], ["TEN", 20.00], ["TWENTY", 60.00], ["ONE HUNDRED", 100.00]],
    register2 = [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.10], ["QUARTER", 4.25], ["ONE", 90.00], ["FIVE", 55.00], ["TEN", 20.00], ["TWENTY", 60.00], ["ONE HUNDRED", 100.00]];

console.log(checkCashRegister(17.46, 20.00, register1));
console.log(register1);

console.log(checkCashRegister(19.50, 20.00, register2));
console.log(register2);

.as-console-wrapper { max-height: 100% !important; top: 0; }

And of course, you could put the coin values in an array too and make a for loop around a single while; but let's not do that work for him :-) — RemcoGerlich, Nov 17 '16 at 09:53
you could streamline it, and i could do it, but i think, the op is learning, first to deal with while loops. — Nina Scholz, Nov 17 '16 at 09:54
i have a decimal error :\ it should return `[["PENNY", 0], ["NICKEL", 0], ["DIME", 0], ["QUARTER", 0.50], ["ONE", 0], ["FIVE", 0], ["TEN", 0], ["TWENTY", 0], ["ONE HUNDRED", 0]]` but it's returning `[["PENNY", 0.04], ["NICKEL", 0], ["DIME", 0.2], ["QUARTER", 0.25], ["ONE", 0], ["FIVE", 0], ["TEN", 0], ["TWENTY", 0], ["ONE HUNDRED", 0]]` — BahaaZidan, Nov 17 '16 at 10:00
i mean when i input this `checkCashRegister(19.50, 20.00, [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.10], ["QUARTER", 4.25], ["ONE", 90.00], ["FIVE", 55.00], ["TEN", 20.00], ["TWENTY", 60.00], ["ONE HUNDRED", 100.00]]);` — BahaaZidan, Nov 17 '16 at 10:16

score 0 · Answer 2 · answered Nov 17 '16 at 10:39

I think you should rethink your code :

function checkCashRegister(price, cash) {
  var change = cash - price;

  var chObj = {
    PENNY: 0,
    NICKEL: 0,
    QUARTER: 0,
    ONE: 0,
    FIVE: 0,
    TEN: 0,
    TWENTY: 0,
    ONE_HUNDRED: 0
  }

  // Performs an euclidian division to get the quotient
  // (in 750$, there are 7 times 100 ie quotient = 7)    
  chObj.ONE_HUNDRED = Math.floor(change / 100);
  // Now get the remainder (750 minus 7 times 100)
  change = change % 100;

  // So on...
  chObj.TWENTY = Math.floor(change / 20);
  change = change % 20;

  chObj.TEN = Math.floor(change / 10);
  change = change % 10;

  chObj.FIVE = Math.floor(change / 5);
  change = change % 5;

  chObj.ONE = Math.floor(change);

  change = Math.floor((change - Math.floor(change)) * 100);

  chObj.QUARTER = Math.floor(change / 20) / 100;
  change = change % 25;

  chObj.NICKEL = Math.floor(change / 10) / 100;
  change = change % 10;

  chObj.NICKEL = Math.floor(change) / 100;

  return chObj;
}

var chObj = checkCashRegister(245, 1000);

for (var key in chObj) {
  if (chObj.hasOwnProperty(key)) {
    console.log(key + " : \t" + chObj[key]);
  }
}

Result (viewable through the console) :

PENNY :     0
NICKEL :    0
QUARTER :   0
ONE :   0
FIVE :  1
TEN :   1
TWENTY :    2
ONE_HUNDRED :   7

It can be more improved i think. See the fiddle here.

This one assumes the cash register has unlimited amounts of each denomination — mplungjan, Nov 17 '16 at 10:49
Yes, but the intention is clear - still kudos for using modulo — mplungjan, Nov 17 '16 at 11:02
In that case I think that `cash` variable is redundant. We only need denomination (plus : there a risk that `cash` won't match the sum of all denominations). Thanks :) — Amessihel, Nov 17 '16 at 11:04
In this case, this code will become a mess if not more optimized. — Amessihel, Nov 17 '16 at 11:27

score 0 · Answer 3 · edited May 23 '17 at 10:30

0

your issue is that you're checking equality to 0 and your actual value is 0.00..[something] you can see that it's a known issue here

change the last if statement to be if (change.toFixed(2) <= 0) {break;}

BTW I suggest you to change all your money calculations to be on cents in order to avoid this kind of issues (instead of 1.0$ use 100 cents).

edited May 23 '17 at 10:30

Community

1
1

answered Nov 17 '16 at 11:34

silver

1,633
1
20
32

score -1 · Answer 4 · answered Nov 17 '16 at 09:57

Here is the reason: the final coin of each kind is never added to the array.

Say the amount is 0.02, 2 cents. Then in the final loop,

while (change - 0.01 > 0) {
  chArr[0][1] += 0.01;
  change -=0.01;
  if (change <= 0) {break;}
}

The guard (0.02 - 0.01 > 0) is true, so the loop is entered and change becomes 0.01.

Next time, 0.01 - 0.01 > 0 is false, so the loop is not entered.

And then the outer loop runs again and again and again, because change is still 0.01 and it never becomes lower.

So that > should have been >=. Then all the breaks as well as the whole outer loop can be removed.

Infinite loop although I put a break;

4 Answers4