Cash Register not returning initial copy of array?

Cash Register not returning initial copy of array?
0

#1

Tell us what’s happening:
I am passing all the tests except the last one. It should return initial copy of the cid parameter but it looks it doesn’t.

I’ve used

let cidCopy = [...cid];

and returning cidCopy,

Your code so far


function checkCashRegister(price, cash, cid) {
  let cidCopy = [...cid];
  let returnArr = [];
  let subtracted = 0;
  var change = cash - price;
  let length;
  let val;
  console.log("change is : " + change)
  // Adding to the array their monetary conversions
  cid[0].push(0.01);
  cid[1].push(0.05);
  cid[2].push(0.1);
  cid[3].push(0.25);
  cid[4].push(1);
  cid[5].push(5);
  cid[6].push(10);
  cid[7].push(20);
  cid[8].push(100);
  
  //Adding to the array how many coins are available for each unit
  for(let i=0; i<cid.length; i++) {
    cid[i].push(Math.round(cid[i][1] / cid[i][2]));
  }


  for(let i=cid.length-1; i>= 0; i--) { //starting from 100 bill to down.
    if(change > cid[i][2]) { //Iterate through every subarray by its value in numbers
          console.log(cid[i])
          length = cid[i][3];
      for(let j=1; j<=length; j++) {
        val = cid[i][2];
        if(change >= val) {
          change -= val;
          change = change.toFixed(2);
          cid[i][3]--;
          cid[i][1] = cid[i][2] * j;
        }
      }
      returnArr.push([cid[i][0], cid[i][1]]);
    }
  }

  console.log(change)
  console.log(change === 0.00);
  console.log(cidCopy)

  if(change < 0.01) change *= 100;

  console.log(change);

  if(change > 0)
      return {
        status : "INSUFFICIENT_FUNDS",
        change : []
      };
  else if(change === 0)
    return {
        status : "CLOSED",
        change : cid
      };

  else
    return {
      status : "OPEN",
      change : returnArr
    };
}

Your browser information:

User Agent is: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 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

What did you get from console.log(change === 0.00)?

I suspect it printed false, did it?

You may be a victim of floating point rounding error and this is a great opportunity to learn about it

For example: 0.1 + 0.2 === 0.3, is this true? Try it and see

So if this is the only problem with the code, (not sure if it is but I suspect so), then you’re probably wondering how to fix this.

If you multiply all the values involved by 100, as in a penny becomes value 1 etc, you can avoid the floating point errors - you just have to be careful about converting them back for the final answer

Edit: Another way you could have found out something was amiss is to wrap checkCashRegister(...) at the end with another console.log, you’d have seen the status being "OPEN"


#3

Thanks, floating point is a good place to start debugging.


#4

the problem is that you are failing the last test right?
That’s because you give a status of OPEN versus CLOSED in the last test.


#5

Alright, I reflected the change to the current code that is being displayed. It is printing out CLOSED but still is returning modified array. Do you know why?


#6

yeah I just realised why

you are using the spread operator to copy cid but that is a shallow copy only.
You need to actually loop through the array of arrays to copy each item properly (like using a map or a foreach)


#7

Ahh yes, I think it may be a case of shallow vs deep copying

cid is a nested array, but spreading it makes a copy of an array of references

Edit: sniped by @hbar1st


#8

Hm ok,

I used below ways to do deep copying and variable cid (The unmodified parameter) is still returning modified array.

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

let cidCopy = cid.map(val => val);

Am I missing something?


#9

The spread operator allows you to copy one level of array

Perhaps you intended to do push(...cid[i]) ?

Edit: why did you do the map afterwards? that doesn’t make sense to me


#10

I didn’t use map afterwards, I just posted it up there to show that one after another didn’t work.

I don’t think I asked my question clearly. I am not trying to return the copy of cid when status is closed but I am just trying to return the initial given cid parameter… but instead it’s returning what looks like cidCopy which has been modified.

    return {
        status : "CLOSED",
        change : cid
      };

#11

well yeah cid has been modified by your code…
you don’t see that?

To make a deep copy of the cid array use this instead

  let cidCopy = [];
  for(let i=0; i<cid.length; i++) {
    cidCopy.push(cid[i].slice());
  }

#12

I think it’s me that’s not being clear, my bad

if you attempt to copy cid into cidCopy with just cidCopy = [...cid] you get an array of references to the inner arrays of cid inside cidCopy

When you modify a subarray of cid you modify it also in cidCopy (and vice-versa)

If you use a method to deep copy cid then modifying either doesn’t affect the other one

This means you have to copy the subarrays, as the spread operator creates whats known as a shallow copy as in it copies the elements of a single level only.


#13

@hbar1st @gebulmer

Thanks guys for your responses, you’ve been very helpful. I think you gave me enough hints for me to figure it out on my own :slight_smile: