Merge two sorted arrays that can be of different sizes

Question

I think the algorithm implementation itself is reasonably efficient, but I'm not sure about proper use of good practices in software development in the code (variable naming, idioms, comments, error handling in inputs etc).

I'm attempting to get a first developer job, and since interviewers also evaluate how "clean" the code is, the main point of the question is what is considered good practice, but efficiency improvements are also welcome.

mergeSortedArrays = function(arr1, arr2){
 if(!(Array.isArray(arr1) && Array.isArray(arr2))){
 throw new TypeError("Need two arrays to merge")
 };
 if(arr2.length === 0 && arr1.length === 0){
 throw new RangeError("Cannot merge empty arrays")
 };
 let i = 0;
 let j = 0;
 const targetSize = arr1.length + arr2.length;
 const mergedArray = [];
 // main loop
 while(mergedArray.length < targetSize){
 /**
 * Works until completion for arrays of the same size
 */
 if(arr1[i] < arr2[j]){
 valueToPush = arr1[i]
 i++
 }else{
 valueToPush = arr2[j]
 j++
 }
 /**
 * For arrays with different sizes, it is safe to push the remainder to the sorted array, given that both inputs are sorted.
 */
 if(valueToPush === undefined){
 const remainingItems = j > arr2.length ? arr1.slice(i) : arr2.slice(j)
 mergedArray.push(...remainingItems)
 break
 }
 mergedArray.push(valueToPush)
 }
 return mergedArray;
}
```

• RoToRa: Both in the title of the post as well as in the code comments, you suggest that this problem were different if the arrays had the same size. However this is not true. Such statements may suggest to interviewers that you didn't completely understand the task.
• Matheus Deister Veiga: Thanks for the comment. That is because a portion of the code that doesn't look right to me was added to handle arrays of different sizes. I was getting [1, 2, 3, undefined] and had to work around it. The size of inputs issue is specific to this implementation, not to the problem itself.
• RoToRa: I'm sorry, but your thinking is wrong. This algorithm you wrote (nor any other reasonable other algorithm) would be different if the arrays had the same size. If you start with one array [1, 2] then it does't matter if the second array is [], [3], [3, 4], or [3, 4, 5]. Any reasonable algorithm would first place the elements 1 and 2 in the result array and then the elements of the seond array. The sizes of the arrays never play any role at all.
• Matheus Deister Veiga: I know it is wrong. That is the point of asking for the code review. The code and the comments look too focused on something that should not have been an issue. A good rephrasing of the question would be: Merge two sorted arrays. And then, on the description: My code reads like I'm solving two separate problems, so I think I'm missing something. Is there a better way to implement it?

Answer 1

I think considering approach that you chose, your code is alright. A few points:

• What I don't like is that you get error when passing 2 empty arrays? Why would you do that? Imagine I am generating arrays of different sizes and using your code to merge and sometimes I would pass 2 empty arrays. Should I really need to handle that as special case in my code? No, just return empty array too.

• I'd eliminate targetSize variable and check for j < arr2.length && i < arr1.length instead.

• Checking valueToPush for undefined feels a bit off for me. It works, but doesn't really feel right. In many languages you would get something like IndexOutOfRange error or something. Maybe it's idiomatic in Javascript (probably not), but I think nicer and cleaner would be to check if you are still in range of array, ex j < arr2.length.

• Matheus Deister Veiga: Thanks for the comments. 1 and 2 are good points. 3 is what made me post this code here. It works, but doesn't look right to me too. I could not come up with a cleaner solution. Checking for if(! ValueToPush) fails because the number 0 evaluates to false in JS, so the code breaks if any of the arrays has a 0. Eliminating the target size and using your index based condition could help me remove this ugly break statement and do the remainder operation after the loop.
• K.H.: Just to be clear, I don't mean just valueToPush == undefined, but whole valueToPush = array1[i]. Your condition basically checks for "is one array already done". I would just do it by checking i against array size instead of checking if you got "undefined" from array. Also your code won't work if there is actually undefined in the array, because your code will recognise it as end of array.

Answer 2

const mergeSortedArrays = (arr1, arr2) => [...arr1, ...arr2].sort((a,b) => a-b);
const ar1 =[-7, 2, 4, 22, 66, 99];
const ar2 = [1, 5, 9, 88];
console.log( mergeSortedArrays(ar1, ar2) );
// [ -7, 1, 2, 4, 5, 9, 22, 66, 88, 99 ]

I don't know if it's not too simple solution, but one-line Arrow function solves this problem...

OR

in your code instead of IF / ELSE "Works until completion for arrays of the same size" you can:

• use shorter form ternary operator ? :
• instantly i++ and j++ - that means you take element arr[i] and arr[j] and

incrementation follows after this operation

valueToPush = ( arr1[i] < arr2[j] ) ? arr1[i++] : arr2[j++]

• Matheus Deister Veiga: Thanks for the comment. It's algorithm interview practice, so using standard functions kind of misses the point, even though on a real life setting, I'd probably be using the arrow function, spread operator and built in sort too. The valueToPush one-liner is pretty cool, didn't know I could increment values inside of array index like this.