Running fine in VS Code but failing in FCC editor [ freeCodeCamp Challenge Guide: Cash Register ]

vikramvi · October 21, 2020, 12:04pm

Tell us what’s happening:

Please find below code which is working fine in VS Code but keep on failing on FCC editor, I have no idea what’s going wrong.

Kindly review and let me know why FCC editor is failing

Your code so far

WARNING

The challenge seed code and/or your solution exceeded the maximum length we can port over from the challenge.

You will need to take an additional step here so the code you wrote presents in an easy to read format.

Please copy/paste all the editor code showing in the challenge from where you just linked.


let returnObject = {
    status: "",
    change: []
}

function calculate(arrElement, baseCurrency, remainingAmount) {
    let localArrayElement = [];

    let cnt1 = arrElement[1] / baseCurrency;

    for (let cnt2 = 0; cnt2 < cnt1; cnt2++) {
        remainingAmount = remainingAmount - baseCurrency;

        remainingAmount = Number(Math.round(remainingAmount + 'e2') + 'e-2');

        console.log(remainingAmount + "   " + baseCurrency);

        if (remainingAmount < baseCurrency) {
            localArrayElement[0] = arrElement[0];
            localArrayElement[1] = baseCurrency * (cnt2 + 1);
            returnObject.change.push(localArrayElement);

            return remainingAmount;
        }
    }

    localArrayElement[0] = arrElement[0];
    localArrayElement[1] = baseCurrency * cnt1;

    returnObject.change.push(localArrayElement);

    return remainingAmount;
}

function checkCashRegister(price, cash, cid) {

    if (cash > price) {
        //let toBePaidBack = cash - price;

        let moneyTBR = cash - price;

        moneyTBR = Number(Math.round(moneyTBR + 'e2') + 'e-2');

        let remainingAmount = moneyTBR;

        //To set status OPEN, CLOSED, INSUFFICIENT_FUNDS
        let totalAmount = 0;
        for (let cnt = 0; cnt < cid.length; cnt++) {
            totalAmount += cid[cnt][1];
        }
        totalAmount = Number(Math.round(totalAmount + 'e2') + 'e-2');

        if (totalAmount < remainingAmount) {

            returnObject.status = "INSUFFICIENT_FUNDS";

            return returnObject;
        }

        for (let cnt = cid.length - 1; cnt >= 0; cnt--) {
            switch (cnt) {
                case 0:
                    if (remainingAmount >= 0.01 && cid[0][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[0], 0.01, remainingAmount);
                    } else if (cid[0][1] === 0) {
                        returnObject.change.push(["PENNY", 0]);
                    }
                    break;
                case 1:
                    if (remainingAmount >= 0.05 && cid[1][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[1], 0.05, remainingAmount);
                    } else if (cid[1][1] === 0) {
                        returnObject.change.push(["NICKEL", 0]);
                    }
                    break;
                case 2:
                    if (remainingAmount >= 0.1 && cid[2][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[2], 0.1, remainingAmount);
                    } else if (cid[2][1] === 0) {
                        returnObject.change.push(["DIME", 0]);
                    }
                    break;
                case 3:
                    if (remainingAmount >= 0.25 && cid[3][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[3], 0.25, remainingAmount);
                    } else if (cid[3][1] === 0) {
                        returnObject.change.push(["QUARTER", 0]);
                    }
                    break;
                case 4:
                    if (remainingAmount >= 1 && cid[4][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[4], 1, remainingAmount);
                    } else if (cid[4][1] === 0) {
                        returnObject.change.push(["ONE", 0]);
                    }
                    break;
                case 5:
                    if (remainingAmount >= 5 && cid[5][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[5], 5, remainingAmount);
                    } else if (cid[5][1] === 0) {
                        returnObject.change.push(["FIVE", 0]);
                    }
                    break;
                case 6:
                    if (remainingAmount >= 10 && cid[6][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[6], 10, remainingAmount);
                    } else if (cid[6][1] === 0) {
                        returnObject.change.push(["TEN", 0]);
                    }
                    break;
                case 7:
                    if (remainingAmount >= 20 && cid[7][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[7], 20, remainingAmount);
                    } else if (cid[7][1] === 0) {
                        returnObject.change.push(["TWENTY", 0]);
                    }
                    break;
                case 8:
                    if (remainingAmount >= 100 && cid[8][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[8], 100, remainingAmount);
                    } else if (cid[8][1] === 0) {
                        returnObject.change.push(["ONE HUNDRED", 0]);
                    }
                    break;
            }
        }

        if (totalAmount === 0) {

            returnObject.status = "CLOSED";

        }

        else if (totalAmount > 0) {

            returnObject.status = "OPEN";

        }

    } else {
        console.log("insufficient cash");
    }

    return returnObject;
}

checkCashRegister(19.5, 20, [["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 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.80 Safari/537.36.

Challenge: Cash Register

Link to the challenge:

JeremyLT · October 21, 2020, 12:30pm

Your code contains global variables that are changed each time the function is run. This means that after each test completes, subsequent tests start with the previous value. To fix this, make sure your function doesn’t change any global variables, and declare/assign variables within the function if they need to be changed.

Example:

var myGlobal = [1];
function returnGlobal(arg) {
  myGlobal.push(arg);
  return myGlobal;
} // unreliable - array gets longer each time the function is run

function returnLocal(arg) {
  var myLocal = [1];
  myLocal.push(arg);
  return myLocal;
} // reliable - always returns an array of length 2

vikramvi · October 21, 2020, 12:48pm

I could get it working with below code

let returnObject = {
    status: "",
    change: []
}

let emptyCurrencyArr = [];

function calculate(arrElement, baseCurrency, remainingAmount) {
    let localArrayElement = [];

    let cnt1 = arrElement[1] / baseCurrency;

    for (let cnt2 = 0; cnt2 < cnt1; cnt2++) {
        remainingAmount = remainingAmount - baseCurrency;

        remainingAmount = Number(Math.round(remainingAmount + 'e2') + 'e-2');

        console.log(remainingAmount + "   " + baseCurrency);

        if (remainingAmount < baseCurrency) {
            localArrayElement[0] = arrElement[0];
            localArrayElement[1] = baseCurrency * (cnt2 + 1);
            returnObject.change.push(localArrayElement);

            return remainingAmount;
        }
    }

    localArrayElement[0] = arrElement[0];
    localArrayElement[1] = baseCurrency * cnt1;

    returnObject.change.push(localArrayElement);

    return remainingAmount;
}

function checkCashRegister(price, cash, cid) {

    if (cash > price) {
        //let toBePaidBack = cash - price;

        let moneyTBR = cash - price;

        moneyTBR = Number(Math.round(moneyTBR + 'e2') + 'e-2');

        let remainingAmount = moneyTBR;

        //To set status OPEN, CLOSED, INSUFFICIENT_FUNDS
        let totalAmount = 0;
        for (let cnt = 0; cnt < cid.length; cnt++) {
            totalAmount += cid[cnt][1];
        }
        totalAmount = Number(Math.round(totalAmount + 'e2') + 'e-2');

        if (totalAmount < remainingAmount) {

            returnObject.status = "INSUFFICIENT_FUNDS";

            return returnObject;
        }

        for (let cnt = cid.length - 1; cnt >= 0; cnt--) {
            switch (cnt) {
                case 0:
                    if (remainingAmount >= 0.01 && cid[0][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[0], 0.01, remainingAmount);
                    } else if (cid[0][1] === 0) {
                        emptyCurrencyArr.unshift(["PENNY", 0]);
                    }
                    break;
                case 1:
                    if (remainingAmount >= 0.05 && cid[1][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[1], 0.05, remainingAmount);
                    } else if (cid[1][1] === 0) {
                        emptyCurrencyArr.unshift(["NICKEL", 0]);
                    }
                    break;
                case 2:
                    if (remainingAmount >= 0.1 && cid[2][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[2], 0.1, remainingAmount);
                    } else if (cid[2][1] === 0) {
                        emptyCurrencyArr.unshift(["DIME", 0]);
                    }
                    break;
                case 3:
                    if (remainingAmount >= 0.25 && cid[3][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[3], 0.25, remainingAmount);
                    } else if (cid[3][1] === 0) {
                        emptyCurrencyArr.unshift(["QUARTER", 0]);
                    }
                    break;
                case 4:
                    if (remainingAmount >= 1 && cid[4][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[4], 1, remainingAmount);
                    } else if (cid[4][1] === 0) {
                        emptyCurrencyArr.unshift(["ONE", 0]);
                    }
                    break;
                case 5:
                    if (remainingAmount >= 5 && cid[5][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[5], 5, remainingAmount);
                    } else if (cid[5][1] === 0) {
                        emptyCurrencyArr.unshift(["FIVE", 0]);
                    }
                    break;
                case 6:
                    if (remainingAmount >= 10 && cid[6][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[6], 10, remainingAmount);
                    } else if (cid[6][1] === 0) {
                        emptyCurrencyArr.unshift(["TEN", 0]);
                    }
                    break;
                case 7:
                    if (remainingAmount >= 20 && cid[7][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[7], 20, remainingAmount);
                    } else if (cid[7][1] === 0) {
                        emptyCurrencyArr.unshift(["TWENTY", 0]);
                    }
                    break;
                case 8:
                    if (remainingAmount >= 100 && cid[8][1] !== 0) {
                        totalAmount = totalAmount - remainingAmount;
                        remainingAmount = calculate(cid[8], 100, remainingAmount);
                    } else if (cid[8][1] === 0) {
                        emptyCurrencyArr.unshift(["ONE HUNDRED", 0]);
                    }
                    break;
            }
        }

        //if (totalAmount > remainingAmount) {
        if (remainingAmount !== 0) {
            returnObject = {
                status: "INSUFFICIENT_FUNDS",
                change: []
            }

        } else {
            if (totalAmount === 0) {

                returnObject.status = "CLOSED";

            }
            else if (totalAmount > 0) {

                returnObject.status = "OPEN";

            }

            returnObject.change.push(...emptyCurrencyArr);
        }

    } else {
        console.log("insufficient cash");
    }


    console.log(returnObject);

    return returnObject;
}

vikramvi · October 21, 2020, 12:51pm

So after spending 8 hours, I could somehow solve this challenge

Later I looked into solutions and most of them have used higher order function reduce.

Kindly clarify below questions

Is it necessary to use higher order function ?
Whose code is good ? with less lines and more complexity or more lines and simpler logic ?
As coding is like making omlette every has got his/her own recipe, how is it determined which is good , bad , ugly code ?
How to make sure ego is not hurt if somebody says this is bad code, make it better. What’s definition , benchmark of better ?

Jr. Dev question,

jsdisco · October 21, 2020, 3:41pm

You made it work without higher order functions, so obviously it’s not necessary to use them. The computer who reads and executes your code won’t care much.

I’d say this depends on the person who reads the code. For a beginner, a for loop is much easier to read than .map, .filter or .reduce. For an experienced coder, those higher order functions are much easier to read than a for loop, because the function’s name already says what it does.

map returns an array of the same length, but its values have changed.
filter returns a subarray of the original array, where only values that met a certain condition are included.
With reduce, it depends on the callback.

A for loop does not give any information about what is happening to the array. I’d have to read the whole loop to get an idea what it does. In your case, I’d even have to read tons of if- and switch-statements to understand what’s going on. Instead of literally writing out each possible case, it would be more readable if there was some abstraction to cover all possible cases with one statement.

Again, this depends on the person who reads the code. Extremely verbose code takes a lot of time to read, extremely concise code takes a lot of time to understand. Finding the sweet spot in between is the key to “good” code.

Well, dealing with critique is necessary everywhere in life.

vikramvi · October 22, 2020, 4:49am

Thanks a ton for your valuable time to reply in detail.

Can you please give your background ?

About me: After working as SDET for 16 years, now I’m shifting to big bad developer world as everyone needs a developer to start with.

P.S.
Strangely in my 16 years of IT career working with India, US and EU based developers; I’ve never found a single developer who said other person has not written spaghetti code sigh

vikramvi · October 22, 2020, 6:20am

@jsdisco I’ve refactoried code can you please do quick review and suggest improvements ?

updated variable names
added comments
removed lengthier switch statements

let returnObject = {
    status: "",
    change: []
}

const currencyUnitWithValue = [["ONE HUNDRED", 100], ["TWENTY", 20], ["TEN", 10], ["FIVE", 5], ["ONE", 1],
["QUARTER", 0.25], ["DIME", 0.1], ["NICKEL", 0.05], ["PENNY", 0.01]];

function giveMeValuesForAmountToBeReturnedBack(arrElement, baseCurrency, moneyToBeReturnedBackToCustomer) {
    let localArrayElement = [];

    let cnt1 = arrElement[1] / baseCurrency;

    for (let cnt2 = 0; cnt2 < cnt1; cnt2++) {
        moneyToBeReturnedBackToCustomer = moneyToBeReturnedBackToCustomer - baseCurrency;

        moneyToBeReturnedBackToCustomer = Number(Math.round(moneyToBeReturnedBackToCustomer + 'e2') + 'e-2');

        console.log(moneyToBeReturnedBackToCustomer + "   " + baseCurrency);

        if (moneyToBeReturnedBackToCustomer < baseCurrency) {
            localArrayElement[0] = arrElement[0];
            localArrayElement[1] = baseCurrency * (cnt2 + 1);
            returnObject.change.push(localArrayElement);

            return moneyToBeReturnedBackToCustomer;
        }
    }

    localArrayElement[0] = arrElement[0];
    localArrayElement[1] = baseCurrency * cnt1;

    returnObject.change.push(localArrayElement);

    return moneyToBeReturnedBackToCustomer;
}

let emptyCurrencyArr = [];

function checkCashRegister(price, cash, cid) {

    if (cash > price) {

        let moneyToBeReturnedBackToCustomer = cash - price;

        moneyToBeReturnedBackToCustomer = Number(Math.round(moneyToBeReturnedBackToCustomer + 'e2') + 'e-2');

        let totalAmountAvailableWithCashier = 0;
        for (let cnt = 0; cnt < cid.length; cnt++) {
            totalAmountAvailableWithCashier += cid[cnt][1];
        }
        totalAmountAvailableWithCashier = Number(Math.round(totalAmountAvailableWithCashier + 'e2') + 'e-2');

        if (totalAmountAvailableWithCashier < moneyToBeReturnedBackToCustomer) {

            returnObject.status = "INSUFFICIENT_FUNDS";

            return returnObject;
        }


        let cnt2 = currencyUnitWithValue.length;

        //loop through available currency values and populate return object
        for (let cnt = 0; cnt < currencyUnitWithValue.length; cnt++) {
            cnt2--;
            if (moneyToBeReturnedBackToCustomer >= currencyUnitWithValue[cnt][1] && cid[cnt2][1] !== 0) {
                //keep on updating amount available with cashier
                totalAmountAvailableWithCashier = totalAmountAvailableWithCashier - moneyToBeReturnedBackToCustomer;

                //keep on updating moeny to be returned back to customer
                moneyToBeReturnedBackToCustomer = giveMeValuesForAmountToBeReturnedBack(cid[cnt2], currencyUnitWithValue[cnt][1], moneyToBeReturnedBackToCustomer);
            } else if (cid[cnt2][1] === 0) {
                emptyCurrencyArr.unshift([currencyUnitWithValue[cnt][0], 0]);
            }
        }


        if (moneyToBeReturnedBackToCustomer !== 0) {
            returnObject = {
                status: "INSUFFICIENT_FUNDS",
                change: []
            }

        } else {
            if (totalAmountAvailableWithCashier === 0) {

                returnObject.status = "CLOSED";

            }
            else if (totalAmountAvailableWithCashier > 0) {

                returnObject.status = "OPEN";

            }

            returnObject.change.push(...emptyCurrencyArr);
        }

    } else {
        console.log("insufficient cash");
    }


    console.log(returnObject);

    return returnObject;
}

ilenia · October 22, 2020, 6:23am

do not use global variables…

vikramvi · October 22, 2020, 6:25am

please clarify why global variables shouldn’t be used ?

Can you share good coding practices for JS based projects ? so that lesser WTF heard during PR code review ?

ilenia · October 22, 2020, 6:52am

have you seen Jeremy post above?

if you use global variables, you get that each function call changes them, making the function output unreliable

even if in the test environment it works if you remove the function call in the editor, it’s bad practice

jsdisco · October 22, 2020, 6:53am

I’m mostly self-taught with some science/engineering background, but development is a real passion.

If someone tells you your code is spaghetti, it’s first of all not more than their opinion, you have the choice to agree with them or not, right? Your code looks much better by the way, it’s definitely improved in readability.

lasjorg · October 22, 2020, 7:13am

Global variables are generally just bad news all around. I’d suggest you read up a bit on functional programming, or at least some of the high notes, like avoiding globals and mutation, using pure functions and composition, and writing declarative code.

You can pick and choose, it doesn’t have to be “hardcore functional”, just functional-ish.

Modern JS code is usually declarative and often takes a more functional style approach.

I would highly recommend learning about the build-in methods and features the language has and trying to write more declarative code. In my opinion, declarative code is far less bug-prone and (for the most) easier to read than imperative code.

I have seen fCC challenge code written imperatively that takes up 60 lines of code that can literally be written in 1 line of code, using built-in methods and language features. Sure you give up some control (mostly the minutiae) and maybe some performance, but I’d say that in 98% of all cases it’s worth it.

I can tell you from personal experience that going from imperative to declarative was the only way I actually ended up liking to write code. The choice between someArray.flat(Infinity) vs 20 lines of C style code is a no brainer for me.

vikramvi · October 22, 2020, 9:56am

and @lasjorg

In the 2nd solution which I’ve shared, I need to keep track of 2 variables namely

moneyToBeReturnedBackToCustomer & returnObject

can you suggest how can I handle this situation as there is no static variable concept in JS & I can only return 1 value from a function

lasjorg · October 22, 2020, 7:02pm

Having two functions both changing the same global variable is a bad idea. In fact, you do not need to mutate a single object, global or function scoped, for this challenge. You can return different objects for each requirement.
The function giveMeValuesForAmountToBeReturnedBack is not doing what it says it does, it is doing more. It is taking input and returning an output, and changing a global data structure. Utility functions should not have side effects. They should be pure, taking input, and returning output.
If you need more than one return value from a function you can use a data structure that supports more than one value, like an object or an array. Then deal with that at the function call site and get to the individual values.
Avoid mutation. Even if you do scope an object to a function, if you pass the object to other functions and change it you will have a mutation. You can make copies and do work on the copy then return the new object.
Using parameters as local variables for storing new values is a bad practice. Use the value in the parameter, but do not change it. Store whatever new value is derived from using the parameter value inside a new variable.
Giving an identifier an extremely verbose name doesn’t help the reader if the usage and current value are hard to follow.

Again, my suggestion is for you to study some functional programming, again nothing crazy. Don’t be put off by all the bizarre lingo used. After you have learned a bit, take some old simple code you have and refactor it to use some functional programming ideas. I think you will find it liberating after the initial adjustment. You have to keep track of far fewer variables constantly changing over time and you will not be juggling as much code as you are now.

Sorry for the long reply, that got away from me a bit.

vikramvi · October 23, 2020, 5:05am

Thanks a ton for detailed clarification point wise, now I completely understand mistakes I’ve made & I’ll rectify those.

I will set aside daily time to understand good programming practice.

Recently I read somewhere programming is like small kid learning new words every day, s/he need as well learn how to put those words in correct sentence otherwise it doesn’t make any sense.

Thanks again for your valuable time & super useful suggestions.

Kind Regards,
Vikram