Merging vectors

Question

I wrote this function that is supposed to merge two vectors in ascending order without duplicates.The function should return the resulting vector, but when I try to display it nothing happens although the display function works well for other vectors. I also don't get any error so it acts like the resulting vector it's empty.I am trying to learn iterators and this is the first time using them so maybe I have misunderstood the concept. Here is the code:

      vector<int> sort(vector<int> &a, vector<int> &b){
         vector <int> sorted;
           for (vector<int>::iterator it = a.begin(); it != a.end();) {
          for (vector<int>::iterator it2 = b.begin(); it2 != b.end();) {
            if (*it < *it2){
               sorted.push_back( *it);
               ++it2;
                }
            else if (*it >*it2){
               sorted.push_back(*it2);
               ++it;
                }
            else if (*it == *it2){
               sorted.push_back(*it);
                ++it;
                ++it2;
                }
          }
       }
        return sorted;
     }

Am I mistakenly using the iterators? Any help is more than appreciated.

The loops here probably don't do what you thought they would do. The inner `for` loop is executed in full for *each* iterator value produced by the outer `for` loop. Also, the function name `sort` is sort of misleading for a merge function. — Cheers and hth. - Alf, Apr 06 '14 at 13:56
Thy not `std::merge`? `sorted.reserve(a.size() + b.size()); std::merge(a.begin(), a.end(), b.begin(), b.end(), std::back_inserter(sorted));` — emsr, Apr 06 '14 at 14:04
Forgot the unique requirement: After merge do `std::unique(sorted.begin(), sorted.end());` then erase from returned iterator to `sorted.end()`. — emsr, Apr 06 '14 at 14:37

score 2 · Answer 1 · answered Apr 06 '14 at 13:58

2

std::set_union defined in <algorithm> does what you want. See here for reference. Under "Possible implementation" you can find a working implementation.

answered Apr 06 '14 at 13:58

Danvil

22,240
19
65
88

score 1 · Answer 2 · answered Apr 06 '14 at 14:00

There are several problems with your code - first, you are using an assignment instead of comparison (i.e. = instead of ==).

More importantly, however, your merge algorithm is wrong: it uses two nested loops instead of a single loop with an AND-ed condition, which works fine only until one of the two vectors runs out of elements. Once this happens, both loops stop, so the "tail" of one of the vectors is going to remain unmerged.

A proper algorithm looks like this:

while (it != a.end() && it2 !=b.end()) {
    ... // Do the merge
}
while (it != a.end()) {
    ... // Push back a's tail
}
while (it2 != b.end()) {
    ... // Push back b's tail
}

Note that there are no nested loops in this solution - the three loops run in succession. Also note that at most two of these three loops will run; either the second or the third loop will be empty.

Note: Obviously, this looks like a learning exercise; if it is not, a better approach would be to use std::merge from the standard C++ library.

score 0 · Answer 3 · edited May 23 '17 at 12:30

0

Why you don't just push all element of the two vector in sorted, and after use sort() function on this? Try to debug:

std::cout << *it << " and " <<*it2 << endl;

EDIT: try:

sort( vec.begin(), vec.end() );
vec.erase( unique( vec.begin(), vec.end() ), vec.end() );

see this post for more details:

What's the most efficient way to erase duplicates and sort a vector?

edited May 23 '17 at 12:30

Community

1
1

answered Apr 06 '14 at 13:58

Julien Leray

1,647
2
18
34

That doesn't do what the OP wanted - it won't filter out duplicates. And it's unlikely to be as efficient. – Ben Hymers Apr 06 '14 at 14:06

Merging vectors

3 Answers3

Linked