Cash Register Project using Map

Code Feedback
1

Hello everyone. I just finished the javascript Cash Register project and I’m happy with the way it turned out. I would like to share my solution with you guys since it seems like it could be valuable. I recently learned about the Map object and discovered that it could be really useful in solving this project.

Let me know what you guys think of it and please let me know of any improvements that I could make:

function checkCashRegister(price, cash, cid) {
  var change = Math.round((cash - price)*100);
  var changeArr = [];
  var moneyVal = {
    "ONE HUNDRED": 10000,
    "TWENTY": 2000,
    "TEN": 1000,
    "FIVE": 500,
    "ONE": 100,
    "QUARTER": 25,
    "DIME": 10,
    "ONE": 100,
    "NICKEL": 5,
    "PENNY": 1
  };
  var changeDrawer = new Map(cid.reverse());
  //multiply everything by 100
  for (var [key, value] of changeDrawer){
    changeDrawer.set(key, Math.round(value*100));
  }

  for (var [key, value] of changeDrawer){
    var tempChangeArr = [];
    while (change >= moneyVal[key] && changeDrawer.get(key) >= moneyVal[key]){
      change -= moneyVal[key];
      changeDrawer.set(key, changeDrawer.get(key) - moneyVal[key]);

      //Add to "status:OPEN" array.
      tempChangeArr[0] = key;
      if (tempChangeArr[1] === undefined) tempChangeArr[1] = 0;
      tempChangeArr[1] += moneyVal[key];
    }
    if (tempChangeArr[1] !== undefined){
      tempChangeArr[1] = tempChangeArr[1] / 100;
    }
    if (tempChangeArr.length > 0){
      changeArr.push(tempChangeArr);
    }
  }

  var sumOfCID = cid.reduce(function(acc, cur){
    return acc + cur[1];
  },0);

  if (change > 0){
    return {status: "INSUFFICIENT_FUNDS", change: []};
  } else if ((cash - price) === sumOfCID){
    return {status: "CLOSED", change: cid.reverse()};
  } else {
      return {status: "OPEN", change: changeArr};
  }

  return change;
}
1 Like
2

You should wrap your code in spoiler tags since it is code for a project required for a certificate.
[SPOILER] some code [/SPOILER]
Example below:

some code

3

My biggest kudos would be two things: Learning about Javascript objects like Map, and your use of descriptive variable names.

My biggest critique would be the use of var, which creates a variable that is accessible anywhere within your function, instead of variables that are scoped only to the their enclosing block. It’s an issue in that first for loop with for(var [key, value] .... for (let [key, value]... would be better, especially since you redeclare the same variable names a few lines down in the second for loop. You don’t want to run into situations where the value of key or value from the first loop interferes with the value from the second loop. Trust me, it’s an common source of bugs that waste time hunting down, and let avoids it.

I like this use of destructuring array and the map.assignment:

  var changeDrawer = new Map(cid.reverse());
  //multiply everything by 100
  for (var [key, value] of changeDrawer){
    changeDrawer.set(key, Math.round(value*100));
  }

But wouldn’t this be more concise & performant (one pass instead of two through the array)?

var changeDrawer = new Map(cid.reverse().map( keyValuePair => [keyValuePair[0], Math.round(keyValuePair[1]*100) );

if you like simpler patterns, use item instead of keyValuePair. I did it for instructional purposes.

1 Like