CODEWITHSUNDEEP
javascriptperformancerecursionrefactoring
JP Douma
JP Douma

Reputation: 135

Is it good practice to use a break statement in this js code snippet?

The purpose of this function is to calculate a price (result) based on a given airline rate table (param 1) and a weight (param 2). The array line in the rates array signify: [weight break, rate/kg, breakpoint].

Example of the calculation:

Note: the last line in the rates table does not have a bp. That's simply how these tables are provided.

TLDR:

My code works just fine (see below), but I am very new at this and I wonder if there is a better way to write this little algorithm. I spot recursion, but I am not sure how to write a recursive function. Perhaps this can be written in a more concise way? I am asking the question to get better at coding. Thanks!

rates = [
    [ 45, 3.8, 88 ],
    [ 100, 3.35, 296 ],
    [ 300, 3.3, 492 ],
    [ 500, 3.25]
];

function calcPrice(arr, weight) {
    let price = 0;
    for (let i = 0; i < arr.length; i++) {
        if (weight <= arr[i][0]) {
            price = arr[i][0] * arr[i][1]; break;
        } else if (weight <= arr[i][2] && arr[i][2] !== undefined) {
            price = weight * arr[i][1]; break;
        } else {
            price = weight * arr[i][1];
        }
    }
    return price;
}
console.log(calcPrice(rates, 89);

Upvotes: 3

Views: 110

Answers (2)

Scott Sauyet
Scott Sauyet

Reputation: 50797

This was tagged with recursion, and I think that's a useful call. A recursive version of this function using destructuring and default parameters ends up being quite simple:

const calcPrice = ([[wb, rate, bp = Infinity], ...rates], weight = 0) =>
  weight < bp 
    ? rate * Math .max (wb, weight) 
    : calcPrice (rates, weight);

const rates = [[45, 3.8, 88], [100, 3.35, 296], [300, 3.3, 492], [500, 3.25]];

const vals = [33, 47, 85, 90, 103, 296, 297, 302, 490, 497, 2001]
vals .forEach (v => console .log (`${v} : ${calcPrice(rates, v).toFixed(2)}`))

Note that by defaulting the breakpoint to Infinity, we can avoid any special checking for the existence of a third entry. Also, the logic about how to do the actual calculation is simplified by noting that we have to compare only with the breakpoint, using a max to choose the value to multiply with our rate. So if the weight is less than the breakpoint, we can return rate * Math .max (wb, weight). If it's not, then we recur on the remaining elements of our array.

Also, your title asked about the use of the break statement. I think your instinct is correct; these should be quite rare. They usually point to code that might do well with a serious refactoring. Besides the recursive technique above, you could also manage this by using something like const [wp, rate, bp = Infinity] = rates.find(([ , , bp]) => weight <= bp). That would also avoid the break statements as well as the explicit for-loop.

Update

This version carries a lot more error-correction by a careful use of default parameters:

const calcPrice = (
  [[weightbreak = 0, rate = 0, breakpoint = Infinity] = [0, 0, Infinity], ...rates] = [], 
  weight = 0
) =>
  weight < breakpoint
    ? rate * Math .max (weightbreak, weight) 
    : calcPrice (rates, weight);

It won't throw an error if the array of rates doesn't exist, or if it's empty, or if some of the values are missing. Instead, it will return zero in most of those circumstances. And it will handle any weight, again using the Infinity to capture large values not yet matched.

Upvotes: 0

Aaron Pettengill
Aaron Pettengill

Reputation: 71

I think there are a couple ways you could make this more concise and readable. It's really just a couple small things.

First one is a JavaScript trick called destructuring assignment. This would allow you to give variable names to the values in the arrays like this:

const [weightBreak, rate, breakpoint] = arr[i];

Then this: arr[i][0] * arr[i][1] becomes weightBreak * rate.

Another small thing is formatting (could just be having to type it into this site). Even if it seems minor, properly formatting your code goes a long way for readability.

Last thing: it looks like that else block could be called on every iteration if we don't find any items in arr where weight <= arr[i][0] or weight <= arr[i][2]. In that case, what we're returning is a value based on the last item in arr. If we use early return statements instead of break statements, we could pull that part out of the loop entirely.

If we put all that together, then we get:

function calcPrice(arr, weight) {
  for (let i = 0; i < arr.length; i++) {
    const [weightBreak, rate, breakpoint] = arr[i];

    if (weight <= weightBreak) {
      return weightBreak * rate;
    }

    if (weight <= breakpoint && breakpoint !== undefined) {
      return weight * rate;
    }
  }

  const lastItem = arr[arr.length - 1];
  return weight * lastItem[1];
}

If you'd like me to clarify any of these, please feel free to ask! I hope that helps and happy coding!

Upvotes: 2

Related Questions