set_interaction example with custom vector not working

Question

I'm looking to get intersection of two custom vectors - v and w - and then deleting common elements from original vector - v. But somehow size of y in my case is 0 always

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

class A{
    public:
    int x;
    A(int i) { x = i; } 
    ~A() {}
    void getx() { cout << x << " "; } 
    friend ostream &operator<<(ostream &o, const A&a) {
        cout << a.x;
        return o;
    }
};

struct mycomparer {
    bool operator()(const A* a, const A* b) {
        return a->x < b->x;
    }
};

int main()
{
    vector<A*> v;
    v.push_back(new A(1));
    v.push_back(new A(2));
    v.push_back(new A(4));
    v.push_back(new A(3));
    v.push_back(new A(5));

    vector<A*> w;
    w.push_back(new A(3));
    w.push_back(new A(2));

    vector<A*> y;
    vector<A*>::iterator it, st;
    it = set_intersection(v.begin(), v.end(), w.begin(), w.end(), y.begin(), mycomparer());

    cout << " y size "  << y.size() << endl;

    if (y.size()) {
        for (st = y.begin(); st != y.end(); st++)
            v.erase(st);
    }

    for (st = v.begin(); st != v.end(); st++) {
        printf("%d ", (*st)->x);
    }
    cout << endl;

    return 0;
}

This is just a sample which I wrote and my intend is not to check for any other C++ rules.

Answer 1

You haven't obeyed the requirements of std::set_intersection.

Constructs a sorted range beginning at d_first consisting of elements that are found in both sorted ranges [first1, last1) and [first2, last2). If some element is found m times in [first1, last1) and n times in [first2, last2), the first std::min(m, n) elements will be copied from the first range to the destination range. The order of equivalent elements is preserved. The resulting range cannot overlap with either of the input ranges.

Neither v nor w are sorted w.r.t mycomparer. That's the first undefined behaviour.

Just passing y.begin() does not mean elements are added to y, for that you need std::back_inserter(y). That's the second undefined behaviour.

You haven't obeyed the requirements of std::vector::erase. It's argument is an iterator into that vector. You are using an iterator from y, not v. That's the third undefined behaviour.

#include <iostream>
#include <vector>
#include <algorithm>
#include <iterator>

class A{
    public:
    int x;
    A(int i) { x = i; } 
    ~A() {}
    void getx() { cout << x << " "; } 
    friend ostream &operator<<(ostream &o, const A&a) {
        cout << a.x;
        return o;
    }
};

struct mycomparer {
    bool operator()(const A* a, const A* b) {
        return a->x < b->x;
    }
};

int main()
{
    std::vector<A*> v;
    v.push_back(new A(1));
    v.push_back(new A(2));
    v.push_back(new A(4));
    v.push_back(new A(3));
    v.push_back(new A(5));

    std::sort(v.begin(), v.end(), mycomparer());

    std::vector<A*> w;
    w.push_back(new A(3));
    w.push_back(new A(2));

    std::sort(w.begin(), w.end(), mycomparer());

    std::vector<A*> y;
    set_intersection(v.begin(), v.end(), w.begin(), w.end(), std::back_inserter(y), mycomparer());

    std::cout << " y size "  << y.size() << std::endl;

    for (st = y.begin(); st != y.end(); st++)
        v.erase(std::find(v.begin(), v.end(), *st));

    for (st = v.begin(); st != v.end(); st++) {
        std::cout << (*st)->x) << std::endl;
    }

    return 0;
}

As an aside, you could instead use std::set_difference to find the elements in v not in w directly.

Answer 2

There are many issues with your code... I just put them in modified code

#include <iostream>
#include <memory>
#include <vector>
#include <algorithm>

// don't using namespace std;

class A {
private:
    int x; // don't expose internals
public:
    constexpr A(int i) noexcept : x(i) {}

    constexpr int GetX() const noexcept { return x; } // don't make a getter to print

    friend std::ostream& operator<< (std::ostream& o, const A& a) {
        o << a.x; // don't push to cout in an ostream operator <<
        return o;
    }
};

// use lambda instead of functors
static constexpr auto mycomparer =
[](std::shared_ptr<A> const& a, std::shared_ptr<A> const& b) noexcept {
        return a->GetX() < b->GetX();
};

int main()
{
    // you were not deleting the new elements: memory leak
    std::vector<std::shared_ptr<A>> v; // e.g. use smart pointers
    v.push_back(std::make_shared<A>(1));
    v.push_back(std::make_shared<A>(2));
    v.push_back(std::make_shared<A>(4));
    v.push_back(std::make_shared<A>(3));
    v.push_back(std::make_shared<A>(5));

    std::vector<std::shared_ptr<A>> w;
    w.push_back(std::make_shared<A>(3));
    w.push_back(std::make_shared<A>(2));

    std::vector<std::shared_ptr<A>> y;

    //you have to sort before calling set_intersection
    std::sort(std::begin(v), std::end(v), mycomparer);
    std::sort(std::begin(w), std::end(w), mycomparer);

    std::set_intersection(
        std::cbegin(v), std::cend(v), // use const iterators
        std::cbegin(w), std::cend(w),
        std::back_inserter(y), mycomparer); // you cannot iterate over an empty array. Use the backinserter

    std::cout << " y size " << y.size() << '\n';

    // you cannot use an iterator to a vector y to delete from vector v!
    //if (y.size() > 0) { // not required
        for (auto const& el : y)
            v.erase(std::find(std::cbegin(v), std::cend(v), el)); 
    //}

    for (auto const& el : v) {
        std::cout << el->GetX() << " ";
    }
    std::cout << '\n'; // prefer \n over endl for speed

    //return 0; // not required
}

Answer 3

1) In vector w you should push new A(2) first, then new A(3). For your actual code, your vectors must be sorted according to the comparator beforehand, as previously mentioned in commentaries, or you absolutely cannot use set_intersection.

2) vector y has size 0, because set_intersection does not insert new elements at iterator, but instead assigns with operator =. Instead of y.begin() you should use std::back_inserter(y), which would actually insert elements on assignment.