3
\$\begingroup\$

I solved this exercise on jshero.com but I know the solution can be written more cleanly, I just don't know how.

Here are the directions:

Write a function addWithSurcharge that adds two amounts with surcharge. For each amount less than or equal to 10, the surcharge is 1. For each amount greater than 10 and less than or equal to 20, the surcharge is 2. For each amount greater than 20, the surcharge is 3.

This challenge also supposes that you use if...else if...else to solve the exercise. The course also hasn't touched on functions within functions, so I'm mainly concerned with simplification and readability, but also curious about ternaries. Here is my attempt which works,

function addWithSurcharge(num1, num2) {
    let surcharge = 0;
    if (num1 <= 10) {
        surcharge += 1;
    } else if (num1 > 10 && num1 <= 20) {
        surcharge += 2;
    } else {
        surcharge += 3;
    }
    
    if (num2 <= 10) {
        surcharge += 1;
    } else if (num2 > 10 && num2 <= 20) {
        surcharge += 2;
    } else {
        surcharge += 3;
    }
    
    return surcharge + num1 + num2;
}

Thank you, much appreciated!

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to CodeReview. The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$
    – Zeta
    Aug 11 '20 at 4:25
3
\$\begingroup\$

From a short review;

  • You are dealing with amounts, but you call your variable num1 and num2
  • Your code now has tons of magic numbers, you should use named constants
  • I understand you are not yet in to 'functions within functions', but that is how you apply DRY in this case

So the DRY version could look like this;

function addSurcharge(amount){
  const FLOOR = 10;
  const CEILING = 20;

  if (amount <= FLOOR) {
    return amount + 1;
  } else if (amount<= CEILING) {
    return amount + 2;
  } else {
    return amount + 3;
  }    
}

function addWithSurcharge(amount1, amount2) {
  return addSurcharge(amount1) + addSurcharge(amount2);
}
\$\endgroup\$
1
\$\begingroup\$

First, why not write a function to compute surcharges? They're the same for both numbers, right? So you would be adding

num1 + surcharge(num1) + num2 + surcharge(num2)

which I think takes care of a lot of the DRY you're looking for.

As for ternary operators, you can use them in a tabular form depending on operator precedence. Something like this:

return num <= 10 ? 1 :
       num <= 20 ? 2 : 
                   3;

It's really one long line, but you're relying on operator precedence to group the second and later ternary expressions together in the else-clause of the first expression.

\$\endgroup\$
0
\$\begingroup\$

Hi if you can use reduce() and ternary operators your code will be much shorter and cleaner. First sums all args and then sums Surcharge.

const addWithSurcharge = (...args) => {
          const sumArgs = args.reduce((a, b) => a + b);
          return args.reduce((acc, cur) => 
          cur <= 10 ? acc + 1 :
          cur <= 20 ? acc + 2 :
          acc + 3, sumArgs);
        }
console.log(addWithSurcharge(10, 30));
console.log(addWithSurcharge(10, 40, 30, 15, 20));

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.