0

I am doing this problem https://leetcode.com/problems/merge-sorted-array/description/

and this is:

You are given two integer arrays nums1 and nums2, sorted in non-decreasing order, and two integers m and n, representing the number of elements in nums1 and nums2 respectively.

Merge nums1 and nums2 into a single array sorted in non-decreasing order.

The final sorted array should not be returned by the function, but instead be stored inside the array nums1. To accommodate this, nums1 has a length of m + n, where the first m elements denote the elements that should be merged, and the last n elements are set to 0 and should be ignored. nums2 has a length of n.

Input: nums1 = [1,2,3,0,0,0], m = 3, nums2 = [2,5,6], n = 3 Output: [1,2,2,3,5,6] Explanation: The arrays we are merging are [1,2,3] and [2,5,6]. The result of the merge is [1,2,2,3,5,6] with the underlined elements coming from nums1.

For input of nums1 =[0] m=0 nums2 = 1 n = 1

enter image description here

i don't know why it is returning [0]

when i am doing nums2 = [...nums1]

can you please help me

/**
 * @param {number[]} nums1
 * @param {number} m
 * @param {number[]} nums2
 * @param {number} n
 * @return {void} Do not return anything, modify nums1 in-place instead.
 */
var merge = function(nums1, m, nums2, n) {
   
    if(m==0){
        nums1 = [...nums2]
        return;
    }
    let placement = nums1.length-1
    let compare1 = m-1
    let compare2 = n-1
    while(compare1>0 && compare2>0){
        if(nums1[compare1]<nums2[compare2]){
            nums1[placement] = nums2[compare2]
            placement -=1
            compare2 -= 1
        }

        if(nums1[compare1]>nums2[compare2]){
            nums1[placement] = nums1[compare1]
            placement -=1
            compare1 -= 1
        }
        if(nums1[compare1] == nums2[compare2]){
            nums1[placement] = nums1[compare1]
            placement -=1
            nums1[placement] = nums2[compare2]
            placement -=1
            compare1 -=1
            compare2 -=1
        }
    }
    return nums1
};
Pravin Poudel
  • 1,433
  • 3
  • 16
  • 38
  • `num1 = [...num2]` doesn't modify the array in place, it instead replaces the array with a new array that is local to the function (the caller can't see it). Also, you're making it needlessly complicated. `const merge = (nums1, m, nums2, n) => { nums1.splice(m, Infinity, ...nums2.slice(0, n)); nums1.sort((a,b) => a-b); };` – Ouroborus Jan 16 '23 at 08:29

1 Answers1

1

The code challenge requires you to mutate the given array nums1, but your code assigns to nums1 (instead of mutating the array it referenced), so now you have lost the possibility to still access and mutate the original array. Although you have a return nums1, the caller does not look at the return value, but will use the reference it has to the original array and see that nothing has changed inside of it. Note the following phrase in the code challenge (I highlight):

The final sorted array should not be returned by the function, but instead be stored inside the array nums1.

Your code has some other issues as well:

  • When the while condition becomes false, it might be that compare2 == 0, which means the value nums2[compare2] has not been consulted yet. So that >0 should really be >=0.

  • When the while condition becomes false, it might be that compare1 < 0, but compare2 is still some positive number, meaning that some values in nums2 have not been moved to nums1. As you want to move all values from nums2 to nums1, the while condition should be compare2 >= 0 only. Then the body of the while loop should check if compare1 >= 0, but this should not be a reason to quit the loop.

    When you have fixed this, there is also no more need to have the special case at the start of the function, and you can remove that if block.

  • The different if blocks should better be mutually exclusive, as otherwise you risk that compare2 becomes negative in the first if block, and although it would work out fine, it is better to just avoid making comparisons with undefined.

  • The last block that checks for equality is overkill. If you make the second block an else to the first block, then there is no more need to check for equality: it would be dealt with in the else block, and the next iterations will take care of the rest.

  • Once you have the if..else, then note that placement-- is always executed exactly once in each iteration of the loop, so it can be moved out of the if..else blocks.

  • Please take the habit to terminate statements with semicolon. Although there is the automatic semicolon insertion procedure, you don't want to make your code's interpretation dependent on the quite complex rules it follows.

Here is your code corrected with the points mentioned above:

var merge = function(nums1, m, nums2, n) {
    let placement = nums1.length - 1;
    let compare1 = m - 1;
    let compare2 = n - 1;
    while (compare2 >= 0) { // Make sure all of nums2 is moved!
        // If nothing more in nums1, then also copy from nums2
        if (compare1 < 0 || nums1[compare1] < nums2[compare2]) {
            nums1[placement] = nums2[compare2];
            compare2 -= 1;
        } else { // Use else, and no specific case for equality
            nums1[placement] = nums1[compare1];
            compare1 -= 1;
        }
        placement -= 1; // This is common to all cases
    }
};
trincot
  • 317,000
  • 35
  • 244
  • 286