1
\$\begingroup\$

I have a sorted array. It can be empty. I have to find its closest greater and smaller number. If the given number is present in the array, it will be considered as greater number. I implemented it like the below style. But I am not satisfied with my approach. Can anyone suggest better ways?

findPreviousNext(array:number[], currentValue: number): ClosestValues
{
    let immediateNextIndex = -1;
    var closestvalues = new ClosestValues();
    for (let i = 0; i < array.length; i++) {
        if (array[i] >= currentValue) {
            immediateNextIndex = i;
            break;
        }
    }
    switch (immediateNextIndex) {
        case 0: {
            closestvalues.immediateNext = array[immediateNextIndex];
            closestvalues.immediatePrevious = null;
            break;
        }
        case -1: {
            var prevVal = array[array.length -1];
            closestvalues.immediateNext = null;
            closestvalues.immediatePrevious = (prevVal != null && prevVal != undefined) ? prevVal : null;
            break;
        }
        default: {
            let prevVal = array[immediateNextIndex - 1];
            closestvalues.immediateNext = array[immediateNextIndex];
            closestvalues.immediatePrevious = (prevVal != null && prevVal != undefined) ? prevVal : null;
        }
}

return closestvalues;
}
New contributor
Shakhawat95 is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
  • \$\begingroup\$ The array is sorted; how about a binary search? \$\endgroup\$ – AJNeufeld Feb 11 at 20:25
  • \$\begingroup\$ Why not move the switch statement into it's own function and have it in the if condition. Or change the for loop to a while loop, drop the break in the if condtion and have the return from closestvalues break the while loop? is this supposed to go through the loop all the way. \$\endgroup\$ – Caperneoignis Feb 11 at 20:29
  • 1
    \$\begingroup\$ @AJNeufeld Please put all suggestions for improvements in answers. \$\endgroup\$ – 200_success Feb 11 at 20:33
  • 1
    \$\begingroup\$ @Caperneoignis Please put all suggestions for improvements in answers. \$\endgroup\$ – 200_success Feb 11 at 20:34
  • \$\begingroup\$ If possible, state the area where you're dissatisfied with your code: embedded documentation, readability, resource usage? \$\endgroup\$ – greybeard Feb 11 at 21:22
1
\$\begingroup\$

First of all, comments would be nice. Speaking in terms of implementation rarely helps another person understand what the code is doing. Especially when the code uses very generic variable names. My general rule to comments is when the code tries to do something clever, write a comment.

You could short-circuit the logic if the array is empty. No sense having the code run if you know ahead of time it does nothing.

Also, by eliminating cases early, you can start making assumptions. This would allow you to simplify logic or make optimizations to existing logic, turning general-case logic to case-specific logic.

for (let i = 0; i < array.length; i++) {
    if (array[i] >= currentValue) {
        immediateNextIndex = i;
        break;
    }
}

Could be simplified using array.findIndex.

Here's my take on it:

const findPreviousNext = (array, currentValue) => {
  // Short-circuit the code if the array is empty.
  if(array.length === 0){
    return { next: null, prev: null }
  }

  // Find the index of the closest greater number.
  const closestIndex = array.findIndex(v => v >= currentValue)

  // A non-existent/negative index is undefined. No need for extensive checks.
  const closestNext = array[closestIndex] || null
  const closestPrev = array[closestIndex - 1] || null
  const lastValue = array[array.length - 1] || null

  // The only special case is the non-existent value (index = -1)
  const next = closestIndex === -1 ? null : closestNext
  const prev = closestIndex === -1 ? lastValue : closestPrev

  // Build the instance (a simple POJO in this case).
  return { next, prev }
}

console.log(findPreviousNext([], 3))
console.log(findPreviousNext([1], 3))
console.log(findPreviousNext([1, 2], 3))
console.log(findPreviousNext([1, 2, 3], 3))
console.log(findPreviousNext([1, 2, 3, 4], 3))

\$\endgroup\$

Your Answer

Shakhawat95 is a new contributor. Be nice, and check out our Code of Conduct.

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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