Freecodecamp: Algorithm Exact Change inaccurate output!

[universse]

Exact Change
https://www.freecodecamp.com/challenges/exact-change

Firefox 47.0
Windows 10

function checkCashRegister(price, cash, cid) {
  var change = cash - price,
      denom = [
        ["ONE HUNDRED", 100.00],
        ["TWENTY", 20.00],
        ["TEN", 10.00],
        ["FIVE", 5.00],
        ["ONE", 1.00],
        ["QUARTER", 0.25],
        ["DIME", 0.10],
        ["NICKEL", 0.05],
        ["PENNY", 0.01]
      ],
      register = cid.reduce(function(acc, curr) {
        acc.TOTAL += curr[1];
        acc[curr[0]] = curr[1];
        return acc;
      }, {"TOTAL": 0});


  // Here is your change, ma'am.

  if (register.TOTAL < change) {
    return "Insufficient Funds";
  } else if (register.TOTAL === change) {
    return "Closed";
  }

  var changeDue = [];
  for (var i = 0; i < denom.length; i++) {
    var currLeft = register[denom[i][0]],
        val = 0;
    while (change >= denom[i][1] && currLeft > 0) {
      change -= denom[i][1];
      currLeft -= denom[i][1];
      val += denom[i][1];
      change = Math.round(change * 100) / 100;
    }


    if (val > 0) {
      changeDue.push([denom[i][0], val]);
    }
  }

  if (change > 0) {
    return "Insufficient Funds";
  }

  return changeDue;

}

checkCashRegister(1, 2.05, [["PENNY", 0], ["NICKEL", 0], ["DIME", 0.30], ["QUARTER", 0.75], ["ONE", 1], ["FIVE", 0], ["TEN", 0], ["TWENTY", 0], ["ONE HUNDRED", 0]]); 

I passed all the FCC's test cases.
However, for the input above, output should be [["DIME", 0.30], ["QUARTER", 0.75]] instead of "Insufficient Fund!".

Similarly, for checkCashRegister(1, 6.05, [["PENNY", 0], ["NICKEL", 0], ["DIME", 0.30], ["QUARTER", 0.75], ["ONE", 1], ["FIVE", 1], ["TEN", 0], ["TWENTY", 0], ["ONE HUNDRED", 0]]); output should be [["DIME", 0.30], ["QUARTER", 0.75], ["FIVE", 1]].

I looked through FCC wiki page for the solution code and still got the same issue.
I'm still trying to solve this challenge taking into account the above cases though.

[BKinahan]

You could of course program a more complex solution that accounts for unrealistic test cases like these, but I think that's beyond the scope of this challenge.

The only reason I can see that cash paid should be more than double (let alone more than six times) the price would be if the customer knows that this is the only way for them to receive exact change is to provide a much larger amount of money than is necessary to cover the price. This doesn't happen in reality because customers are rarely informed of the exact contents of a cash register for various reasons (especially before a first attempt to pay) 😄 It's also easier for customers to verify they are given correct change if they don't grossly overpay.

So the realistic case would be paying the price value or just over it to a nearest approximation in the first instance, and the cashier attempting to make change, which is what this challenge simulates. In reality the cashier and customer could then have a dialogue such as, "Oh, I don't have the right change for that, but if you have a nickel I can give you a dollar in change," for example.

[universse]

It's not exactly unrealistic actually. Those are just some examples that I used to illustrate my point. I completely overlooked the fact that they were unrealistic. My bad!
However, let's say the price is $8.95 and cash is $10, which is more realistic I guess. Thus, the change is still $1.05 and the above code would still give incorrect output.
Or maybe I'm just overthinking haha.

[BKinahan]

@blueirisss That is indeed more reasonable! It is certainly realistic and does require a more complex solution.

So the question up for discussion is whether a test case should be added to make the challenge more complex and difficult in order to make it a more accurate approximation of the problem represented.

At this point in the course, is it reasonable to expect users to learn how to generate the various possible combinations of change to see if it can be paid correctly, rather than simply attempting to pay as much as possible with the highest denominations downward?

[dskloet]

Instead you could make it explicit in the problem statement that the cashier will always try the maximum amount of each denomination and may fail to give change even though it is possible. "Insufficient funds" would have to be changed to "Failed to give change" or something.

[QuincyLarson]

@dskloet @universse are either of you still interested in improving this challenge to cover this corner case?

[dskloet]

@QuincyLarson I'm not sure what you're asking for but I have a correct solution that's quite readable that I'm happy to share.

[GeniaT]

Hello, not sure if I should open another issue but... I got for this challenge various test cases that are not met even though my output seems correct.

1) One "Insufficient Funds" isn't met while my output is correct in both challenge scenarios.
2) It says my output isn't an array when actually it is (tested via instanceof)
3) For the case when [["QUARTER", 0.50]] is wanted, I got [["QUARTER", 0.5]] and wonder if it should be set as correct or not. The rest is secondary.

Here is my code and the result from it (it's not clean but before going on solution version, I want to understand from mine):

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

  // calculate the value in cid
  function addTwoValues (arr1, arr2) {
    var suming = arr1 + arr2[1]; //arr1 is 0 at first then it become var suming. arr2[1] is the value in each subarray like 1.01 in ["PENNY", 1.01]
    return suming;

  }

```
var cidSum = cid.reduce(addTwoValues, 0); //returns results like 3.0599999999 so need a rounding rule at this pt
var cidSumRounded = Math.round(cidSum * 100) / 100; //Not perfect rule but enough for this case as we want to round at 2 decimals

//If the change due > what's in the cid, return "Insufficient funds".
if (changeDue === cidSumRounded) {
console.log("Closed");
return "Closed";
} else if (changeDue > cidSumRounded) {
console.log("Insufficent Funds");
return "Insufficient Funds";
} else {
change(cid);
}

  ```
//with the changeDue amount,, return from highest to lowest the bills/coins + amount that represent the change. Or "Insufficient Funds" if the exact change can't be given.
  function change(cid) {
    var change = [];
    var coinValues = [0.01, 0.05, 0.1, 0.25, 1, 5, 10, 20, 100];

    //Add value of bills and coins to array cid so I have one array with the needed info for calculation
    for (var i = 0; i < 9; i++) {
      cid[i].push(coinValues[i]);
    }

//fct with loop to run through all arr for change Due calculation
function changeCalcul(arr) {
  var changeDue = cash - price; //Need to redeclare and associate the value for changeDue

  //updating the change array in ["PENNY", 0,4] format
  function updateChange () {
    change.push(arr[i].slice(0,1));
    change[change.length-1].push(valueInCoin);
  }

  for (var i = 8; i >= 0; i--) { //we start from last arr(biggest coin)

    if (arr[i][2] <= changeDue) { //if the coin value doesn't exceed changeDue

          if (changeDue > arr[i][1]) { //and changeDue is greater than the amount available in this coins
            //var changeDue & valueInCoin can't be in higher scope as they depend on nbrCoins
            var nbrCoins = arr[i][1] / arr[i][2];
            var valueInCoin = Math.floor(nbrCoins) * arr[i][2]; //alternative to %.
            var changeDue = Math.round((changeDue - valueInCoin) * 100) / 100; //to avoid values like 34,7899999999, converting to 34,79
            updateChange();

          } else if (changeDue <= arr[i][1]) { //changeDue < the amount available in this coins
            var nbrCoins = changeDue / arr[i][2];
            var valueInCoin = Math.floor(nbrCoins) * arr[i][2];
            var changeDue = Math.round((changeDue - valueInCoin) * 100) / 100;
            updateChange();
          }

    }

    if (changeDue === 0) { //we stop scanning other arrays the change is correct
    console.log(change);
    return change;
    }
  }
  //If at the end of the loop, we still have a changeDue, there isn't enough money in the cid!
  if (changeDue > 0) {
    console.log("Insufficent Funds");
    return "Insufficient Funds";
  }
}
changeCalcul(cid);

} }

I know my code is dirty here but I find weird to fail so many challenge tests with this version when outputing them in the browser looks fine...

Do you see anything that may explain it or should the challenge checks be reviewed?
Thanks a lot,

[dskloet]

I'm happy to help, but I'm not sure what your question is.
Can you ask a specific question with a specific example?

[GeniaT]

@dskloet very sorry, I spent 6 hours on this today >_<... I updated my comment with 3 points. Hope it's more clear for you, thanks again !

[dskloet]

I still don't understand your questions, but I had a look at your code and it seems you just forgot to put return on line 21 and line 79.

[GeniaT]

@dskloet That changes nothing unfortunately. My point is that 3 checks fail according to the challenge while my code gives the right answer when I console.log it before returning.

Was wondering if there are some checks missing in the challenge or not.

[dskloet]

@GeniaT if I copy your code into the challenge page and add return on line 21 and 79, it passes all the tests for me.

I've properly indented your code and added the 2 returns here:

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

  // calculate the value in cid
  function addTwoValues (arr1, arr2) {
    var suming = arr1 + arr2[1]; //arr1 is 0 at first then it become var suming. arr2[1] is the value in each subarray like 1.01 in ["PENNY", 1.01]
    return suming;

  }
  var cidSum = cid.reduce(addTwoValues, 0); //returns results like 3.0599999999 so need a rounding rule at this pt
  var cidSumRounded = Math.round(cidSum * 100) / 100; //Not perfect rule but enough for this case as we want to round at 2 decimals

  //If the change due > what's in the cid, return "Insufficient funds".
  if (changeDue === cidSumRounded) {
    console.log("Closed");
    return "Closed";
  } else if (changeDue > cidSumRounded) {
    console.log("Insufficent Funds");
    return "Insufficient Funds";
  } else {
    return change(cid);
  }
  //with the changeDue amount,, return from highest to lowest the bills/coins + amount that represent the change. Or "Insufficient Funds" if the exact change can't be given.
  function change(cid) {
    var change = [];
    var coinValues = [0.01, 0.05, 0.1, 0.25, 1, 5, 10, 20, 100];

    //Add value of bills and coins to array cid so I have one array with the needed info for calculation
    for (var i = 0; i < 9; i++) {
      cid[i].push(coinValues[i]);
    }
    //fct with loop to run through all arr for change Due calculation
    function changeCalcul(arr) {
      var changeDue = cash - price; //Need to redeclare and associate the value for changeDue

      //updating the change array in ["PENNY", 0,4] format
      function updateChange () {
        change.push(arr[i].slice(0,1));
        change[change.length-1].push(valueInCoin);
      }

      for (var i = 8; i >= 0; i--) { //we start from last arr(biggest coin)

        if (arr[i][2] <= changeDue) { //if the coin value doesn't exceed changeDue

          if (changeDue > arr[i][1]) { //and changeDue is greater than the amount available in this coins
            //var changeDue & valueInCoin can't be in higher scope as they depend on nbrCoins
            var nbrCoins = arr[i][1] / arr[i][2];
            var valueInCoin = Math.floor(nbrCoins) * arr[i][2]; //alternative to %.
            var changeDue = Math.round((changeDue - valueInCoin) * 100) / 100; //to avoid values like 34,7899999999, converting to 34,79
            updateChange();

          } else if (changeDue <= arr[i][1]) { //changeDue < the amount available in this coins
            var nbrCoins = changeDue / arr[i][2];
            var valueInCoin = Math.floor(nbrCoins) * arr[i][2];
            var changeDue = Math.round((changeDue - valueInCoin) * 100) / 100;
            updateChange();
          }

        }

        if (changeDue === 0) { //we stop scanning other arrays the change is correct
          console.log(change);
          if (change instanceof Array) { //verifying if it's an array
            alert('value is Array!');
          } else {
            alert('Not an array');
          }
          return change;
        }
      }
      //If at the end of the loop, we still have a changeDue, there isn't enough money in the cid!
      if (changeDue > 0) {
        console.log("Insufficent Funds");
        return "Insufficient Funds";

      }
    }
    return changeCalcul(cid);
  }
}

[GeniaT]

@dskloet I was checking on wrong spot, my bad. Thanks a lot ! Sorry for the big spam...

[dskloet]

No worries. Glad your problem is solved now.

[johntyree]

My own two cents as an experienced developer is that this problem teaches newcomers that it's totally normal and acceptable for your functions to return completely different types depending on arbitrary input conditions. I know javascript is not statically typed, but this is the stuff that bugs are made of.

I'd suggest having it at least tag the return value with the type in some way, such as {registerState: 'CLOSED', change: [...]} or {registerState: "INSUFFICIENT_FUNDS", change: []} etc.

As-is, the correct solution wouldn't pass code review at any of the places I've worked. Seems in bad taste to teach it to people that are trying to pass interviews someday.

[QuincyLarson]

@johntyree thank you for your feedback.

I agree that even though JS isn't statically typed, we should teach learners to have these return a similar data structure regardless of the result.

Would you be willing to help us update this challenge, its tests, and its instructions? We would welcome a PR from you.

[johntyree]

Somehow I didn't notice your reply! Sorry about that. I'll open a new bug to track the reworking of the question.