How can I Implement a better version of my function find the closest number to zero?

Question

I recently had to do a test for a job interview, and the prompt was to code a function that returns the closest number to 0 given an array of negative and positive numbers.

My function works correctly but I heard back from the recruiter saying that my function is not very performant and that the readability was not great, here are the problem that they mentioned:

Performances à double check (filter + sort + for + push + reserve + shift, 4 identical call to Math.abs) Lack of readability ( 4 if / else if) I would like to know how I could have improved my function to make it more performant and more readable?

here's my function:

const closestToZero = (arr) => {
 // 1-a variables:
 let positiveNumbers = [];
 let negativeNumbers = [];
 // 1-b returns 0 if the input is either undefined or an empty array
 if(typeof(arr) === 'undefined' || arr.length === 0) {
 return 0
 }
 // 2- filter out non-number values then sort the array
 const onlyNumbers = arr.filter(item => typeof(item) === 'number').sort((a,b) => a-b);
 // 3- split the numbers into positive numbers and negative numbers
 for(let i = 0; i < onlyNumbers.length; i++) {
 if(onlyNumbers[i] > 0) {
 positiveNumbers.push(onlyNumbers[i])
 } else if (onlyNumbers[i] < 0) {
 negativeNumbers.push(onlyNumbers[i])
 }
 }
 // 4- reverse the negative array to get the closest to 0 at the first index
 let reversedNegativeArray = negativeNumbers.reverse()
 // 5- get rid of all the other values and keep only the closest to 0, if array empty return 0
 let closestPositiveNumber = positiveNumbers.length > 0 ? positiveNumbers.shift() : 0
 let closestNegativeNumber = reversedNegativeArray.length > 0 ? reversedNegativeArray.shift() : 0
 // 6- Compare the result of the substraction of the closestPositiveNumber and the closestNegativeNumber to find the closestToZero
 if(closestPositiveNumber - Math.abs(closestNegativeNumber) > 0 && closestNegativeNumber === 0 ) {
 return closestPositiveNumber
 } else if (closestPositiveNumber - Math.abs(closestNegativeNumber) < 0 && closestPositiveNumber === 0) {
 return closestNegativeNumber
 } else if(closestPositiveNumber - Math.abs(closestNegativeNumber) > 0) {
 return closestNegativeNumber
 } else if (closestPositiveNumber - Math.abs(closestNegativeNumber) <= 0) {
 return closestPositiveNumber
 }
}

requirements:

• if the closest number in input could be either negative or positive, the function returns the positive one
• if the input array is undefined or empty, the function returns 0

when input is [8, 5, 10] the function returns 5

when input is [5, 4, -9, 6, -10, -1, 8] the function returns -1

when input is [8, 2, 3, -2] the functions return 2

Answer 1

Your reviewers are correct. You are doing way too much work here. To find the number closest to zero you only need to find the smallest absolute value in the list. You should also clarify in the interview if the list can contain non-numeric values. There's no reason to test that the values are numbers unless you need to for the purpose of the review, although asking about this shows that you are thinking about edge cases, which is good.

Either way, this can be done in a single loop with no storage other than keeping track of the current minimum:

function absSmallest(arr){
 if (!arr || arr.length === 0 ) return 0 // address requirement to return 0 when arr is undefined or empty
 let res = undefined // smallest value seen so far
 for (let i = 0; i < arr.length; i++){
 if (res === undefined || Math.abs(arr[i]) <= Math.abs(res)){
 res = Math.abs(arr[i]) === Math.abs(res) // address requirement of positive value when there's a tie 
 ? Math.max(res, arr[i] )
 : res = arr[i]
 }
 }
 return res
}
console.log(absSmallest([5, 4, -9, 6, -10, -1, 8] ))
console.log(absSmallest([8, -2, 3, 2] ))
// check order of tied values
console.log(absSmallest([8, 2, 3, -2] ))
// edge case empty list returns 0:
console.log(absSmallest())
console.log(absSmallest([]))

• Roland Illig: The code can be made even simpler using for (const num of arr). Did you choose the longer form intentionally?
• Sᴀᴍ Onᴇᴌᴀ: That code doesn't satisfy the second requirement: "if the input array is undefined or empty, the function returns 0"
• Mark M: @SᴀᴍOnᴇᴌᴀ that requirement was added later. It's easy enough to satisfy, I'll edit.
• Mark M: @RolandIllig yes I chose the longer for form to use the idiom the poster was using.

Answer 2

From the advise of MarkM. Finding the min of absolute values of the array. A bit more readable than his approach (personal opinion).

function closestToZero(arr) {
 if (!arr || arr.length === 0) {
 return 0;
 }
 let closestToZero = arr[0];
 arr.forEach(function(number) {
 if (Math.abs(number) < Math.abs(closestToZero)) {
 closestToZero = number;
 }
 });
 return closestToZero;
}

Answer 3

I would like to point out few things in your code:-

1. This should be at the first place of your function.

   if(typeof(arr) === 'undefined' || arr.length === 0) { return 0 }

2. You have used arrow function here which is of no use here, as according to rules if I tell you, arrow function must be used in one liner function.

3. You could have not used anonymous function or have used class.

4. Hardest part here, what you could have done is just find the element next to 0 and before 0 after sorting it, compare there diff with 0, which has least that is the closest element.