0

Dear StackOverflow community,

Having the type Couple defined like this:

class Couple
{
public:
    Couple(Thing* aa, Thing* bb) : a(aa), b(bb) {};
public:
    bool operator == (const Couple& rhs) const { return this->a->unique_id == rhs.a->unique_id && this->b->unique_id == rhs.b->unique_id; }
    bool operator != (const Couple& rhs) const { return this->a->unique_id != rhs.a->unique_id || this->b->unique_id != rhs.b->unique_id; }
    bool operator <= (const Couple& rhs) const { return this->a->unique_id <  rhs.a->unique_id || this->a->unique_id == rhs.a->unique_id && this->b->unique_id <= rhs.b->unique_id; }
    bool operator >= (const Couple& rhs) const { return this->a->unique_id >  rhs.a->unique_id || this->a->unique_id == rhs.a->unique_id && this->b->unique_id >= rhs.b->unique_id; }
    bool operator <  (const Couple& rhs) const { return this->a->unique_id <  rhs.a->unique_id || this->a->unique_id == rhs.a->unique_id && this->b->unique_id <  rhs.b->unique_id; }
    bool operator >  (const Couple& rhs) const { return this->a->unique_id >  rhs.a->unique_id || this->a->unique_id == rhs.a->unique_id && this->b->unique_id >  rhs.b->unique_id; }
    operator Couple() const { return *this; }
public:
    Thing* a;
    Thing* b;
};

I'm trying to use c++ std::find function:

std::vector<Couple*> generate_couples(std::vector<Thing*> things)
{
    std::vector<Couple*> couples;

    for (unsigned int i = 0; i < things.size(); i++)
    {
        for (unsigned int j = 0; j < things.size(); j++)
        {
            if (things[i] != things[j] && match(things[i], things[j]))
            {
                Couple  c = Couple(things[i], things[j]);
                Couple rc = Couple(things[j], things[i]);

                auto it_c  = std::find(couples.begin(), couples.end(),  c);
                auto it_rc = std::find(couples.begin(), couples.end(), rc);
                if (it_c == couples.end() && it_rc == couples.end())
                    couples.push_back(&c);
            }
        }
    }

    return couples;
}

Visual Studio Community 2017 gives an Error:

Error C2679 binary '==': no operator found which takes a right-hand operand of type 'const _Ty' (or there is no acceptable conversion)

originated from line 3520 of the file

c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.16.27023\include\xutility

I've overloaded all the comparison operators, including '==', I've been trying to overload conevrsion operator as it's noted in the last part of an error. I've also tried to define operator == outside the class and declaring it as a friend function:

bool operator == (const Couple& lhs, const Couple& rhs)
{ return lhs.a->unique_id == rhs.a->unique_id && lhs.b->unique_id == rhs.b->unique_id; }
class Couple
{
    // ...
    friend bool operator == (const Couple& lhs, const Couple& rhs); 
    // ...
};

and it raises an error:

E0344 too many parameters for this operator function

There are several related questions, the most similar one is this (How to use std::find/std::find_if with a vector of custom class objects?), but I couldn't find solution to my problem there.

Thanks in advance for any comments and answers!

Lev Pleshkov
  • 363
  • 1
  • 14
  • 1
    `couples.push_back(&c);` -- Storing an address of a local variable like this is undefined behavior. What happens on the next iteration of the loop, when that `c` you have the address of either gets recycled, or goes up in a puff of smoke and disappears? – PaulMcKenzie Jul 19 '20 at 08:57
  • Unrelated: `operator Couple() const { return *this; }` will never be used so you can remove it. – Ted Lyngmo Jul 19 '20 at 08:59
  • 1
    Also, but not related, when overloading relational operators, all you really need to fully implement are operators `<` and `==`. All the other ones can be implemented in terms of `<` and `==`. The reason to do things this way is that if `<` and `==` are complex, you're not trying to figure out how to turn things inside out to implement `!=`, `>=`, etc. For example: `bool operator != (const Couple& rhs) const { return !(*this == rhs); }` is much easier than possibly having a bug trying to invert `==` the hard way. – PaulMcKenzie Jul 19 '20 at 09:01
  • [Here is an example of the relational operators](http://coliru.stacked-crooked.com/a/4096a0f955f248fc). Note the simplification of all of those other operators, instead of trying to figure out inverses. – PaulMcKenzie Jul 19 '20 at 09:14
  • Wow, thanks, @PaulMcKenzie! It makes sense :) – Lev Pleshkov Jul 19 '20 at 13:16

2 Answers2

2

So I think you have a few problems, but the most fundamental is this

std::vector<Couple*> couples;

You have a vector of Couple pointers. no amount of overloading operators for the Couple class is going to help you find things in vector of pointers.

Another problem you have is that you are pushing pointers to locally scoped objects onto your vector. When those objects go out of scope (which happens each time round the loop) they are destroyed leaving pointers to invalid objects in your vector.

So the obvious first step would be to use a vector<Couple> instead of a vector<Couple*>. If you feel you have to use a vector of pointers then you are going to have to start allocating objects with new and use some variety of find_if as MakeCAT suggests.

I have no explanation for the too many parameters for this operator function error. That should not have happened from the posted code. So probably you made some mistake which isn't apparent from the code above.

john
  • 85,011
  • 4
  • 57
  • 81
  • Thanks for your detailed explanation! I did thought that ```vector``` may cause problems, but was focused on the error and was blind to it.. And I think I do need to return a vector of pointers from this function. So if I dynamically allocate ```Couple``` objects with operator ```new```, the pointers in the vector should remain valid after the program leaves the scope of this function, shouldn't they? – Lev Pleshkov Jul 19 '20 at 13:24
  • @LevPleshkov That's correct, they will remain valid until you delete them. The danger of course is either that you'll never delete them, or that you will delete them more than once. It's for reasons like that that using smart pointers instead of raw pointers is recommended. With a smart pointer the delete happens automatically. – john Jul 19 '20 at 14:20
1

You are using vectors of pointers to Couple, so operators defined in Couple are not directly used from std::find.

You can use std::find_if to have it dereference the pointers and use operators in Couple.

auto it_c  = std::find_if(couples.begin(), couples.end(), [&](auto p){ return *p == c; });
auto it_rc = std::find_if(couples.begin(), couples.end(), [&](auto p){ return *p == rc; });
MikeCAT
  • 73,922
  • 11
  • 45
  • 70