Overloading == Operator

[alexyoung1992]

Hi,

Here I have a class:

class point{
public:
    const point& operator=(const point&);
    bool operator==(const point&) const;
    bool operator!=(const point&) const;
    
    point();
    point(int x, int y);
    
    int get_x() const;
    void set_x(int);
    int get_y() const;
    void set_y(int);
    
private:
    int X, Y;
};

And the == operator overload is defined as follows:

bool point::operator==(const point& p) const{
    return(this->X==p.get_x() && this->Y==p.get_y());
}

It compiles fine but I keep getting a run time segmentation fault every time I run it through the debugger. Anyone know why? Thanks.

[Zhuge]

Show the code you are using to test the operator.

[alexyoung1992]

I am using the == operator in another class defined as:

class cell{
public:
    cell();
    cell(point&);
    virtual ~cell();
    
    point getCoord() const;
    void setCoord(point&);
    void setCoord(int, int);
    virtual cell_type getType()=0;
    virtual void print(std::ostream &)=0;
    
protected:
    point coord;
    cell_type type;
};

I am using this method:

void cell::setCoord(point &coord){      
    this->coord=coord;
}

which calls:

const point& point::operator=(const point& p){
    if(*this!=p){
        this->X=p.X;
        this->Y=p.Y;
    }
    return *this;
}

which calls:

bool point::operator!=(const point& p) const{
    return !((*this)==p);
}

which then calls the == operator.

[Disch]

That setCoord function is using the = operator, not the == operator.

Where, exactly, is this segfaulting? What makes you think it has to do with the == operator?

[alexyoung1992]

Well, the = operator uses the != operator which uses the == operator, so indirectly, setCoord function uses the == operator. And I ran my code through a debugger, and it told me that it received a segmentation fault at this line of code return(this->X==p.get_x() && this->Y==p.get_y());.

Quick edit:
if I redefine the != operator as

bool point::operator!=(const point& p) const{
    return (this->X!=p.get_x() && this->Y!=p.get_y());
}

I now get a segmentation fault at that line of code.

One more edit:
I tried just taking out the if statement in my = operator definition to see if it would work, and now I get a segmentation fault at line 4 of:

const point& point::operator=(const point& p){
//    if(*this!=p)
    {
        this->X=p.get_x();
        this->Y=p.get_y();
    }
    return *this;
}

[Disch]

My money is on a bad 'this' pointer.

void cell::setCoord(point &coord){      
    this->coord=coord;
}

What code calls this function?


Also, doing if(*this!=p){ in your assignment operator is completely pointless. It's more work to check to see if the two objects are equivilent than it would be to just unconditionally assign them.

[alexyoung1992]

This calls the setCoord function:

void generate_map(int height, int width, vector<vector<cell*> > &grid){
    grid.resize(height);
    for(int i=0; i<height; i++){
        grid[i].resize(width);
    }
    for(int i=0; i<height; i++){
        for(int j=0; j<width; j++){
            point p(i,j);
            grid[i][j]->setCoord(p);
        }
    }
}

And thanks for the tip on the if statement, I thought it would be a good precaution, but good to know its unnecessary.

Also, I edited the following functions as such:

void cell::setCoord(point &_coord){      
    coord=_coord;
}

const point& point::operator=(const point& p){
    X=p.get_x();
    Y=p.get_y();
    return *this;
}

And I still get a segfault at X=p.get_x();

Also, I was under the impression that it is a good coding practice for when you are implementing a method that uses member variables of a class to precede the member variable with this->. Is this not the case? If not, when and when should I not do it?

[Disch]

Okay that's your problem. You're creating a bunch of pointers, but those pointers don't point to anything. Therefore you're trying to assign objects that don't exist.


Get rid of the pointer nonsense. You don't need it here.

void generate_map(int height, int width, vector<vector<cell> > &grid){  // <- cell, not cell*
    grid.resize(height);
    for(int i=0; i<height; i++){
        grid[i].resize(width);
    }
    for(int i=0; i<height; i++){
        for(int j=0; j<width; j++){
            point p(i,j);
            grid[i][j].setCoord(p);  // <-
        }
    }
}

Don't use pointers unless there's a reason to. They make things more complicated than they often need to be.

[alexyoung1992]

Ok that makes total sense. I didn't have pointers initially, but the only reason why I changed it to pointers was because, ultimately I have several sub classes that I want to use:

class Player : public cell{
public:
    Player();
    ~Player();
    virtual cell_type getType() const;
    virtual void print(std::ostream &) const;
};

class Beast : public cell{
public:
    Beast();
    ~Beast();
    virtual cell_type getType() const;
    virtual void print(std::ostream &) const;
};

class Movable : public cell{
public:
    Movable();
    ~Movable();
    virtual cell_type getType() const;
    virtual void print(std::ostream &) const;
};

class Immovable : public cell{
public:
    Immovable();
    ~Immovable();
    virtual cell_type getType() const;
    virtual void print(std::ostream &) const;
};

class Empty : public cell{
public:
    Empty();
    ~Empty();
    virtual cell_type getType() const;
    virtual void print(std::ostream &) const;
};

I wanted to create a 2D vector of generic cells, and then based on what the user inputted, change the cells to a more specific kind of cell. I thought I could only typecast a base class into a derived class through the use of pointers, no?

[Disch]

Ah, yes. In that case you will need to use pointers.

But then you'll need to actually create objects (with new) and initialize those pointers before you try to dereference them.