-1

I am trying to override the == operator for a class, however the comparison seems to failing somehow. when I write the same as a function called eq(for example) no problem occurs.

class geo
{
    ...
        bool operator==(geo const& other)
        {
            if(_id != other._id) return false;
            if(!(name == other.name))return false; 
            if(is_primary!=other.is_primary)return false;
            for(int i = 0; i<6;i++){
                if(position[i]!=other.position[i])
                    return false;
            }
            return true;
        }
   ....
    private:
        int _id, is_primary;
        vector position;
        string name;

}

In main function: ...

geo* i= new geo(2.1, 2.82, 1, 0, 0, 180, 0, "Patient-1",1);
geo* j= new geo(2.1, 2.82, 1, 0, 0, 180, 0, "Patient-1",1);
if(i==j)
    std::cout<<"they are equal\n";

However when I run this, it says they are different for i and j. Any idea of where i do the stuff wrong ?

Edit: Thank you guyz at the comments. I just solved it; the code shown above works pretty ok. If course I was trying to simplify the code to make here paste something readable. So I am updating the code above to turn it into a problem so that future readers might see a better solution than I did, as well as I can learn more.

sariug
  • 106
  • 7
  • 1
    Please provide constructor declaration, for geo ? and "vector position" is syntactically valid?? – sha111 Jul 01 '19 at 04:08
  • 1
    "Any idea of where i do the stuff wrong ?" - yeah, there's a problem in the code you didn't show. – M.M Jul 01 '19 at 04:10
  • 1
    debug it with putting break point on each return false statement and check where you return false – samini Jul 01 '19 at 04:19
  • The code as shown looks correct (assuming the `position` vector always has exactly 6 items in it -- are you sure you didn't meant to use `i – Jeremy Friesner Jul 01 '19 at 04:31
  • Btw you probably want to add the `const` qualifier to the end of your `==` operator-declaration (i.e. `bool operator==(geo const& other) const {...}`), otherwise you won't be able to check a const-tagged `geo` object for equality. – Jeremy Friesner Jul 01 '19 at 04:32
  • Can you provide the definition of 'position'? I think the == operator may be incorrect, which is leading to the array comparison returning true when it shouldn't. – jignatius Jul 01 '19 at 04:56
  • guys I am sorry that I actually wrote a valid code here while simplifying it. I updated in a way that its problematic to learn your perspectives. – sariug Jul 01 '19 at 05:08
  • In the main function you are comparing the pointers of `i` and `j` instead of comparing the objects themselves. In reference to `position[i]!=other.position[i]`, you'd need to compare floating-point numbers with tolerance. – Sussch Jul 01 '19 at 05:24
  • use `if(*i==*j)` instead – TaQuangTu Jul 01 '19 at 05:27
  • 1
    Are you coming from Java? You should almost never use pointers and `new` in C++. – n. m. could be an AI Jul 01 '19 at 05:32
  • @n.m. "You should almost never use pointers and new in C++" saying that can you clarify how it actually should be ? (assuming you are not speaking of anything related also to smart pointers.) – sariug Jul 01 '19 at 06:37
  • 1
    *When and if* you need *owning/managing* pointers you should use smart pointers. A raw pointer is not managing, and should be never initialised with `new` but always with an address of an existing object. Normally you just use objects. `geo i(2.1, 2.82, 1, 0, 0, 180, 0, "Patient-1",1);`. – n. m. could be an AI Jul 01 '19 at 09:15

2 Answers2

6

By doing i == j you are comparing two pointers to geo, and not the objects they are pointing to. Since the pointers are obviously different, you get the result as such.

To actually compare objects, you need to dereference the pointers:

if (*i == *j) `

CinCout
  • 9,486
  • 12
  • 49
  • 67
0

It looks to me as if your geo class's comparison operator is a lexographical compare of its members.

You may be interested to know that you can use a tuple to unify all comparison operators by building a std::tie of the members in the appropriate order.

Example:

#include <string>
#include <vector>
#include <tuple>
#include <iostream>

class geo
{
public:
    template<class...Args>
    geo(Args&&...);

private:
    auto as_tuple() const { 
        // note: mentioned in lexographical order
        return std::tie(_id, name, is_primary, position); 
    }

    friend auto operator ==(geo const& l, geo const& r) -> bool
    {
        return l.as_tuple() == r.as_tuple();
    }

    friend auto operator <(geo const& l, geo const& r) -> bool
    {
        return l.as_tuple() < r.as_tuple();
    }

private:
    int _id, is_primary;
    std::vector<int> position;
    std::string name;
};


int main()
{
    geo i= geo(2.1, 2.82, 1, 0, 0, 180, 0, "Patient-1",1);
    geo j= geo(2.1, 2.82, 1, 0, 0, 180, 0, "Patient-1",1);

    if(i==j) {
        std::cout<<"they are equal\n";
    }

    if(i < j) {
        std::cout<<"i is less\n";
    }
}
Richard Hodges
  • 68,278
  • 7
  • 90
  • 142