1
#include <iostream>
#include<vector>
std::vector<int> print(std::vector<int> answerVec){
    for(int i = 0; i < answerVec.size(); i++){
        std::cout<<answerVec.at(i)<<" ";
    }
    std::cout<<"\n";
    return answerVec;
}
std::vector<int> Merge(std::vector<int> left, std::vector<int> right){
    int leftSize = left.size();
    int rightSize = right.size();
    std::vector<int> finalVector;
    int leftIterator = 0, rightIterator = 0;
    while(leftIterator < leftSize && rightIterator < rightSize){
        //std::cout<<"Left"<<left.at(leftIterator)<<"\n";
        //std::cout<<"Right"<<right.at(rightIterator)<<"\n";
        if((int)left.at(leftIterator) <= (int)right.at(leftIterator)){
            finalVector.push_back(left.at(leftIterator));
            leftIterator ++;
        }
        else{
            finalVector.push_back(right.at(rightIterator));
            rightIterator++;
        }
        //print(finalVector);
    }
    while(leftIterator < leftSize){
        finalVector.push_back(left.at(leftIterator));
        leftIterator ++;
        //print(finalVector);
    }
    while(rightIterator < rightSize){
        finalVector.push_back(right.at(rightIterator));
        rightIterator++;
        //print(finalVector);
    }
    return finalVector;
}
int main(){
    std::vector<int> vecL,vecR,vec;
    int n,input;
    std::cin>>n;
    for(int i = 0; i < n; i++){
        std::cin>>input;
        vecL.push_back(input);
    }
    for(int i = 0; i < n; i ++){
        std::cin>>input;
        vecR.push_back(input);
    }
    vec = Merge(vecL, vecR);
    for(int i = 0; i < vec.size(); i++){
        std::cout<<vec.at(i)<<" ";
    }
    return 0;
}

The Merge function above is not functioning properly when I am passing two sorted vectors as argument. Have tried for a while now but cannot make it work. Have commented some print statements that I was using for debugging. Any help would be much appreciated.

Thanks in advance

1 Answers1

1
if((int)left.at(leftIterator) <= (int)right.at(leftIterator)){

Here, the second leftIterator should be rightIterator.

Gassa
  • 8,546
  • 3
  • 29
  • 49
  • 1
    One of those days when you want to bang your head against the wall after seeing your mistake...Thanks – Sandeep Walia Apr 05 '18 at 10:23
  • @SandeepWalia The good next question is how to prevent this kind of stupid bug, but in this particular case, I sadly don't have a compelling answer. – Gassa Apr 05 '18 at 10:25
  • One fix might be to use const iterators instead of indices? That way left and right are only mentioned once. – Gem Taylor Apr 05 '18 at 10:32
  • As a minor refinement, please reserve on your final vector to save it resizing? – Gem Taylor Apr 05 '18 at 10:33
  • @GemTaylor Ah, right! The real [iterator](https://stackoverflow.com/q/2395275/1488799) (as opposed to integer index) is one mention of both the vector and the position in it. – Gassa Apr 05 '18 at 10:44
  • Thanks guys still a newbie when it comes to c++. Have completed my final implementation of merge sort using vectors (link - http://cpp.sh/4fipb). Can you guys help me know whats wrong. Apologies in advance if it is a stupid mistake. – Sandeep Walia Apr 05 '18 at 11:04
  • @SandeepWalia: Your implementation of merge sort is recursive. Each recursion step stores its result in the global variable `finalAnswer`, which will be overwritten repeatedly. Each step should have its own state. – M Oehm Apr 05 '18 at 11:15
  • @MOehm Do you mean that I should store the final merge answer in a new vector. Can you modify my code and share better implementation – Sandeep Walia Apr 05 '18 at 11:21
  • @SandeepWalia: You could return the new arrays from each call ([cpp.sh link](http://cpp.sh/8yesh)), but you will end up passing a lot of vectors around and you will create a lot of new temporary vectors. Alternatively,you could sort the array in place ([cpp.sh link](http://cpp.sh/8lhma)). You still need to create a scratch array for merging, though. Note that here,you have to pass the vecor as reference (`&`),so that the contents of the original vactor are modified. – M Oehm Apr 05 '18 at 11:55
  • @MOehm Thanks a ton for the solutions. One thing that I can't get my head around is that what is wrong logically with my code. I do get that constantly overwriting finalAnswer vector is bad, but why is the code not sorting the array correctly? – Sandeep Walia Apr 05 '18 at 13:10
  • Perhaps you could stop thinking in terms of actual vectors, and instead consider using a pair{const_iterator,const_iterator} to represent a range as the input. This would get rid of some input copies. – Gem Taylor Apr 05 '18 at 13:55