JS: Cash Register

Code Feedback
1

Tell us what’s happening:

Alright I feel like this code is too entangled to work with. I have passed all but one test and I doubt I’ll be able to pass it with this code. Should I reset the project and start over or do you see hope in this code?

Your code so far


function checkCashRegister(price, cash, cid) {
  // Global Usage
  let statusArray = ["INSUFFICIENT_FUNDS", "CLOSED", "OPEN"];
  let currency = [0.01, 0.05, 0.1, 0.25, 1, 5, 10, 20, 100];

  let change = (cash * 100) - (price * 100);
  let unchangedChange = change;

  let cashInRegister = [...cid];

  let totalCash = 0;
  for(let i = 0; i < cid.length; i++) {
      totalCash += (cid[i][1]) * 100;
      currency[i] *= 100;
      cashInRegister[i][1] *= 100;
  }

  console.log(currency);

  function pushChange() {
    let localRegister = [...cashInRegister];
    for(let i = 0; i < localRegister.length; i++) {
      localRegister[i][1] /= 100;
    }

    receipt.change = [...localRegister];
  }
  
  let receipt = {
    status: "",
    change : []
  };

  if(change > totalCash) {
    receipt.status = statusArray[0];
    receipt.change = [];
  } else if(change == totalCash) {
    receipt.status = statusArray[1];
    pushChange();
  } else {
    
    let matchesGreaterThanChange = [];
    let currencyUsed = [];
    let anotherArray = [];
    for(let i = cashInRegister.length - 1; i >= 0; i--) {
      if(change >= currency[i] && cashInRegister[i][1] != 0) {
      console.log("Change in question: " + cashInRegister[i][1] / 100);
        while(change >= currency[i] && cashInRegister[i][1] >= 0) {
          if(cashInRegister[i][1] != 0) {
            currencyUsed.push(cashInRegister[i][1] / 100);
          }
          pusherFunction(change, i);
          console.log("Cash: " + cashInRegister[i][1] / 100);
          console.log("Currency: " + currency[i] / 100)
          console.log("Change: " + change / 100);
          console.log();
          change -= currency[i];
          cashInRegister[i][1] -= currency[i];
          if(cashInRegister[i][1] == 0) {
            anotherArray.push(currencyUsed[0]);
            currencyUsed = [];
          } else {
            
          }
          matchesGreaterThanChange.push(cashInRegister[i][1]);
        }
      }
    }

    console.log(currencyUsed);
    console.log(anotherArray);
    function pusherFunction(changeInput, index) {
      if(changeInput == currency[index] && currency[index] != currency[0]) {
        let toBePushed = [];
        toBePushed.push(cashInRegister[index][0]);
        toBePushed.push(unchangedChange / 100);
        receipt.change.push(toBePushed);
      }
    }

    if(change > matchesGreaterThanChange[matchesGreaterThanChange.length - 1]) {
      receipt.status = statusArray[0];
      receipt.change = [];
    } else {
      receipt.status = statusArray[2];
    }
  }
  console.log(totalCash);
  console.log("Change: " + change);
  console.log(receipt.status + " and " + receipt.change);
  console.log(receipt);
  // Here is your change, ma'am.
  return receipt;
}

checkCashRegister(3.26, 100, [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.1], ["QUARTER", 4.25], ["ONE", 90], ["FIVE", 55], ["TEN", 20], ["TWENTY", 60], ["ONE HUNDRED", 100]]);

Your browser information:

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.121 Safari/537.36.

Link to the challenge:
https://learn.freecodecamp.org/javascript-algorithms-and-data-structures/javascript-algorithms-and-data-structures-projects/cash-register/

2

Sometimes starting from scratch can be helpful, but I think you’re pretty close here and it’s less than 100 lines of code, so I would say try refactoring what you have.

Here are a few tips:

  • put your helper functions at the bottom (or even better outside of your main function) and try to have the main function read as an outline to what you’re doing.
  • try to avoid nesting a loops and conditionals too deep. In this case you should not need to go more than 2 levels deep. As I commented on your other thread, the inner while loop is not necessary.
  • You are converting dollars to pennies and back in too many places. You should only need to do that in 3 places at most.
3

These should start to sound familiar:

  • pick names carefully (e.g. pusherFunction is not very clear, don’t mix i and index, ideally try to avoid both :slight_smile: - see below)
  • avoid relying on array indices too much, e.g. instead of
const currency = [0.01, 0.05, 0.1, 0.25, 1, 5, 10, 20, 100];

try to use:

  const currencyValues = {
    'PENNY': 0.01,
    'NICKEL': 0.05,
    'DIME': 0.1,
    'QUARTER': 0.25,
    'ONE': 1,
    'FIVE': 5,
    'TEN': 10,
    'TWENTY': 20,
    'ONE HUNDRED': 100,
  };

and reference this object by unit

4

Haha yeah.

hmm. that’s a good thought. Honestly I’m almost there but somehow everything is intertwined one value changes everything fails. I have a lot of refactoring to do.

5

I refactored the entire code and I have it working now.

function fixRoundingError(input, action) {
  if(Array.isArray(input)) {
    for(let i = 0; i < input.length; i++) {
      if(action === "multiply") {
        input[i][1] *= 100;
        input[i][1] = Math.round(input[i][1]);
      } else if (action === "divide") {
        input[i][1] /= 100;
      }
    }
  } else if (typeof input === "object") {
    for(let property in input) {
      if(action === "multiply") {
        input[property] *= 100;
      } else if(action === "divide") {
        input[property] /= 100;
      }
    }
  } else if(typeof input === "number"){
    if(action === "multiply") {
      input *= 100;
    } else if(action === "divide") {
      input /= 100;
    }
  }

  return input;
}

function checkCashRegister(price, cash, cid) {
  // Global Usage
  cid[cid.length - 1][0] = "ONE_HUNDRED";

  const statusValues = ["INSUFFICIENT_FUNDS", "CLOSED", "OPEN"];
  const currencyValues = {
    PENNY: 0.01,
    NICKEL: 0.05,
    DIME: 0.1,
    QUARTER: 0.25,
    ONE: 1,
    FIVE: 5,
    TEN: 10,
    TWENTY: 20,
    ONE_HUNDRED: 100
  };

  let receipt = {
    status: "",
    change : []
  };
  
  // params
  let multiplyParam = "multiply";
  let divideParam = "divide";

  // Rounding up values
  fixRoundingError(currencyValues, multiplyParam);
  fixRoundingError(cid, multiplyParam);

  // Can't round up cash and price as is. They are localized.
  let localPrice = fixRoundingError(price, multiplyParam);
  let localCash = fixRoundingError(cash, multiplyParam);
  let change = localCash - localPrice;

  let totalCash = 0;
  for(let i = 0; i < cid.length; i++) {
      totalCash += (cid[i][1]);
  }

  if(change > totalCash) {
    receipt.status = statusValues[0];
  } else if(change == totalCash) {
    receipt.status = statusValues[1];
    cid[cid.length - 1][0] = "ONE HUNDRED";
    receipt.change = fixRoundingError(cid, divideParam);
  } else {
    
    for(let i = cid.length - 1; i >= 0; i--) {

      let currencyName = Object.keys(currencyValues)[Object.keys(currencyValues).indexOf(cid[i][0])];
      var currencyUsed = [currencyName, 0];
      while((change >= cid[i][1] || change >= currencyValues[cid[i][0]]) && cid[i][1] != 0) {
        currencyUsed[1] += currencyValues[currencyName];
        change -= currencyValues[currencyName];
        cid[i][1] -= currencyValues[currencyName];
      }

      if(currencyUsed[1] > 0) {
        currencyUsed[1] = fixRoundingError(currencyUsed[1], divideParam);
        receipt.change.push(currencyUsed);
      }
    }

    if(change > 0 && change != totalCash) {
      receipt.status = statusValues[0];
      receipt.change = [];
    } else {
      receipt.status = statusValues[2];
    }
  }
  // Here is your change, ma'am.
  return receipt;
}
  
checkCashRegister(3.26, 100, [["PENNY", 1.01], ["NICKEL", 2.05], ["DIME", 3.1], ["QUARTER", 4.25], ["ONE", 90], ["FIVE", 55], ["TEN", 20], ["TWENTY", 60], ["ONE HUNDRED", 100]]);

I guess I was intimidated by the floating numbers and was frantically trying to come up with solutions everywhere. Thank you very much. I have tried to implement all of your suggestions wherever I possibly could.

1 Like