3
\$\begingroup\$

I have a function that takes a specific number and a range, and re-scales it to the corresponding number in a different range.

/**
 * @param   int     number      The number to adjust.
 * @param   int     oldMin      The old minimum value.
 * @param   int     oldMax      The old maximum value.
 * @param   int     newMin      The new minimum value to convert to.
 * @param   int     newMax      The new maximium value to convert to.
 * 
 * @returns int     The original number converted to the new boundary.
*/
function convertValueToBoundary(number, oldMin, oldMax, newMin, newMax)
{
    return (((number - oldMin) * (newMax - newMin)) / (oldMax - oldMin)) + newMin
}

Formula

newValue = (((oldValue - oldMinimum) * (newMaximum - newMinimum)) / (oldMaximum - oldMinimum)) + newMinimum

Example

For instance, given the number 5 within a range of 0-10, we want to increase the numerical range to be between 0-100 instead.

convertValueToBoundary(5, 0, 10, 0, 100) // Output -> 50

Optimization & Improvements

Is this the optimal way to conduct this calculation? After a number of different attempts the above formula is the best I could come up with. Given a scenario where this needs to execute multiple times in rapid succession, it would be ideal to have this execute with the least amount of arithmetic operations required.

The conversion should be conducted without the use of any native language functions as I plan to use the same formula across different languages and platforms.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ some languages implement faster/easier methods to do certain computations, although not really necessary in your case, but for eg. you can check matrix operations in APL. \$\endgroup\$
    – hjpotter92
    Oct 27, 2021 at 5:54
  • 2
    \$\begingroup\$ This could still do with additional context. For example, if we're rescaling lots of numbers between the same two ranges, we might want to save the subtractions somewhere (and perhaps take a functional approach, where we return a function that performs one particular rescale). When you port to different languages, it may well be worth having the new implementations reviewed - especially if you might be using integers, which behave very differently to Javascript's floating-point numbers. \$\endgroup\$ Oct 27, 2021 at 6:50

1 Answer 1

2
\$\begingroup\$

Code noise

Your naming and code style make the code very noisy.

The function name is too long convertValueToBoundary the conversion is implied and value is commonly shortened to val so valToBoundary is less noisy. However I would never guess what this function did without some extra information

Maybe rescale would be more appropriate.

You have too many brackets. You have...

return (((number - oldMin) * (newMax - newMin)) / (oldMax - oldMin)) + newMin

is identical to...

return (number - oldMin) * (newMax - newMin) / (oldMax - oldMin) + newMin;

To further remove code noise use an arrow function and shorten the argument names

const rescale = (val, oMin, oMax, min, max)=>(val - oMin) * (max - min) / (oMax - oMin) + min;

Optimization

The only optimization possible is to remove the division (multiplication is quicker than division). This would improve the performance if you were converting many values from the same range...

const rescale = (val, oMin, invRange, min, max)=>(val - oMin) * (max - min) * invRange + min;

// Your example convertValueToBoundary(5, 0, 10, 0, 100) // Output -> 50
// would be
rescale(5, 0, 0.1, 0, 100); // result 50;  is quicker
// or
rescale(5, 0, 1 / 10, 0, 100); // result 50; is slower as need the / and *

However if each range is different this would make the operation slower as you need to calculate the inverse range each time you call the function.

You could also use two ranges to remove the subtraction making it even quicker

const rescale = (val, oMin, invRange, min, range)=>(val - oMin) * range * invRange + min;

rescale(5, 0, 0.1, 0, 100); // result 50

Guard

If you keep the function with the division you need to guard against divide by zero. In JS convertValueToBoundary(5, 0, 0, 0, 100) will result in NaN rather than the expected 0 and in some languages it will throw an error.

Thus you should have the guard oMin === oMax to prevent the divide by zero.

const rescale = (val, oMin, oMax, min, max) => 
    (oMin === oMax ? 0 : (val - oMin) * (max - min) / (oMax - oMin)) + min;

If you use inverse range there is no need for the guard.

const rescale = (val, oMin, invRange, min, max)=>(val - oMin) * (max - min) * invRange + min;

// or

const rescale = (val, oMin, invRange, min, range)=>(val - oMin) * range * invRange + min;

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback, especially regarding the division! \$\endgroup\$
    – Skully
    Oct 27, 2021 at 18:25

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.