c++sortingdata-structuresmerge

Debugging algorithm that merges two lists that are already sorted using circular shifts


Could you help me with debugging this C++ code involving the merge algorithm explained below? I suspect that the index pointers are causing the bug. Below is the paragraph with the required specifications.

Assignment specifications: The following problem asks us to merge two lists (x1, ... , xm) and (x_(m+1), ..., x_n) that are already sorted, into a sorted list (x_l, ..., x_n). Let s = floor(sqrt(n)). Assume that one of these lists has fewer than s records and the first list is shorter than the second. We are asked to write a function that merges the two sorted lists. This merge function must use the circularShiftRight() function which shifts the records circularly to the right by p positions. If the first list has fewer than s elements, then we should find the position, q, in the merged list of the first element of the first list. Then we should perform a circular shift of q - 1 positions by using circularShiftRight() function. This circular shift involves only the records of the first list and the first q - 1 records of the second. Following the circular shift, the first q records are in their final merged positions. Repeat this process for the second, third, and remaining elements of the initial first list.

This is the MRE below:

#include <cmath>
#include <iostream>

class Element {
  public:
    int getKey() const { return key; }
    void setKey(int k) { key = k; }    
    Element(int k = 0) : key(k) {}
  private:
    int key;
};

void reverseSegment(Element* arr, int start, int end) {
    while (start < end) {
        int tempKey = arr[start].getKey();
        arr[start].setKey(arr[end].getKey());
        arr[end].setKey(tempKey);
        start++;
        end--;
    }
}

void circularShiftRight(Element* arr, int n, int p) {
    if (n <= 1 || p == 0) return;
    
    p = p % n;
    if (p == 0) return;
    
    reverseSegment(arr, 0, n - 1);
    reverseSegment(arr, 0, p - 1);
    reverseSegment(arr, p, n - 1);
}

void printArray(Element* arr, int n) {
    std::cout << "Array: ";
    for (int i = 0; i < n; i++)
        std::cout << arr[i].getKey() << " ";
    std::cout << std::endl;
}

void mergeWithShift(Element* arr, int start1, int len1, int start2, int len2) {
    int n = len1 + len2;
    int s = static_cast<int>(floor(sqrt(n)));
    
    std::cout << "sqrt(n) = " << s << std::endl;
    std::cout << "First list length = " << len1 << std::endl;
    
    if (len1 <= s) {
        std::cout << "Processing: First list is shorter than or equal to sqrt(n)" << std::endl;
        
        for (int i = 0; i < len1; i++) {
            int currentElement = arr[start1 + i].getKey();
            std::cout << "\nProcessing element " << currentElement << " from position " << (start1 + i) << std::endl;
            
            // Find position q where current element belongs by comparing with second list
            int q = start2;
            while (q < start2 + len2 && arr[q].getKey() < currentElement) {
                q++;
            }
            
            std::cout << "Element " << currentElement << " should move to before position " << q << std::endl;
            
            if (q > start1 + i) {
                // Calculate number of positions to involve in the shift
                // This includes elements from current position to q - 1
                int shiftRange = q - (start1 + i);
                
                std::cout << "Performing circular shift on range [" << (start1 + i) << ", " << (q-1) 
                         << "] right by " << shiftRange - 1 << " positions" << std::endl;
                
                // circularShiftRight on the range from current position to q - 1
                // The shift amount is (shiftRange - 1) to move current element to position q - 1
                circularShiftRight(arr + start1 + i, shiftRange, shiftRange - 1);
            }
            
            std::cout << "Array after processing element " << currentElement << ": ";
            printArray(arr, n);
        }
    } else {
        std::cout << "First list is longer than sqrt(n)" << std::endl;
    }
}

int main() {
    Element arr[10] = {1, 4, 7, 2, 3, 5, 6, 8, 9, 10};
    int start1 = 0, len1 = 3; // First list: {1, 4, 7}
    int start2 = 3, len2 = 7; // Second list: {2, 3, 5, 6, 8, 9, 10}
    
    std::cout << "Original ";
    printArray(arr, len1 + len2);
    
    mergeWithShift(arr, start1, len1, start2, len2);
    
    std::cout << "Merged ";
    printArray(arr, len1 + len2);
    
    return 0;
}

The output is shown below:

Test Case 1:
Original Array: 1 4 7 2 3 5 6 8 9 10
sqrt(n) = 3
First list length = 3
Processing: First list is shorter than or equal to sqrt(n)

Processing element 1 from position 0
Element 1 should move to before position 3
Performing circular shift on range [0, 2] right by 2 positions
Array after processing element 1: Array: 4 7 1 2 3 5 6 8 9 10 

Processing element 7 from position 1
Element 7 should move to before position 7
Performing circular shift on range [1, 6] right by 5 positions
Array after processing element 7: Array: 4 1 2 3 5 6 7 8 9 10 

Processing element 2 from position 2
Element 2 should move to before position 3
Performing circular shift on range [2, 2] right by 0 positions
Array after processing element 2: Array: 4 1 2 3 5 6 7 8 9 10 
Merged Array: 4 1 2 3 5 6 7 8 9 10

The result 4 1 2 3 5 6 7 8 9 10 was supposed to be 1 2 3 4 5 6 7 8 9 10.


Solution

  • First of all, the algorithm description has a wrong instruction when it says "we should perform a circular shift of q - 1 positions". They defined q as the "the position in the merged list", so not counting from the very left, but from the start of the second list. This is also clear from the phrase "the first q - 1 records of the second". But that means that the size of the range that is shifted is greater than q, since we should include the elements of the first partition to the shift. But if then we only move q - 1 positions to the right, the targeted element does not necessarily end up at its desired index. It is as if the authors suddenly defined q as the index from the very left of the whole array. For the algorithm to work, the shift should bring the left most value to its sorted position in the second partition. If q represents the index in the overall array, then it is correct to say that q elements should be shifted q-1 times to the right.

    Here is how your example input could be processed, step by step:

    We have the two initial partitions:

     (1   4   7) (2   3   5   6   8   9   10)
    

    Then we identify the place where 1 should end up in the second partition, which is just before the 2 (indicated with an arrow):

     (1   4   7) (2   3   5   6   8   9   10)
                ↑
    

    Now the 1 should be rotated into that position. The shift will involve all values at the left of the arrow. And once that shift completes, the 1 will have become part of the second partition:

       Rotated
     -----------
     (4   7) (1   2   3   5   6   8   9   10)
    

    We then continue with value 4, which should end up between 3 and 5 in the second list:

     (4   7) (1   2   3   5   6   8   9   10)
                        ↑
    

    4 should be rotated to that position:

           Rotated
     -------------------
     (7) (1   2   3   4   5   6   8   9   10)
    

    The last value to process is 7, and it is to be placed between 6 and 8:

     (7) (1   2   3   4   5   6   8   9   10)
                                ↑
    

    The final rotation makes that happen:

                Rotated
     ---------------------------
     (1   2   3   4   5   6   7   8   9   10)
    

    And now the second partition is the only one that remains, and it is sorted.

    Some issues in your approach

    Not a problem, but as start1 is always 0, and start2 is always len1, I would reduce the number of variables and parameters you have in your code, and just suffice with n and len1 for that purpose.

    Here is the suggested adapted code:

    void mergeWithShift(Element* arr, int n, int len1) {
        int s = static_cast<int>(floor(sqrt(n)));
        if (len1 > s) {
            std::cout << "First list is longer than sqrt(n). Merge aborted." << std::endl;
            return;
        }
    
        while (len1 > 0) { // The first partition shrinks in each iteration
            int currentElement = arr[0].getKey(); // Get the now left-most element
            // Find index in the second list before which this element should be rotated 
            int q = len1;
            while (q < n && arr[q].getKey() < currentElement) {
                q++;
            }
            // The shift always starts at index 0: rotate the range [0, q-1]
            circularShiftRight(arr, q, q - 1);
            len1--; // The merged (second) partition has grown.
        }
    }
    
    int main() {
        Element arr[10] = {1, 4, 7, 2, 3, 5, 6, 8, 9, 10};
        int n = 10; // Total length of both partitions together
        int len1 = 3; // First partition: {1, 4, 7}
        std::cout << "Original ";
        printArray(arr, n);
        mergeWithShift(arr, n, len1);
        std::cout << "Merged ";
        printArray(arr, n);
        return 0;
    }
    

    If we should assume that the shift operation is a black-box operation that could execute in constant time, then you can improve the efficiency by replacing the inner while loop with a binary search.