Disadvantages of "has-a" relationship, C++

Question

I have a situation similar to the following.

class Point
{
    void setCoord(int x, int y)
    {
        this->x = x;
        this->y = y;
    }

    private:
        int x,y;
}

class Line
{
    public:
        Point *p1;
        Point *p2;
}

The Line constructor and destructor look like

Line::Line()
{
    p1 = new Point();
    p2 = new Point();
}

Line::~Line()
{
    delete p1;
    delete p2;    
}

Now, in the application can call methods of the inner object Point.

int main()
{
    Line line;
    line.p1->setCoord(x1,y1);
    line.p2->setCoord(x2,y2);
}

I have received some suggestions that 1) "has-a" relationship is bad and that 2) the pointers to the p1 and p2 and can be replaced.

I agree with the 2nd point is valid. Can we prevent the pointers being replaced by the application?

However, I am not convinced about the 1st point. Is "has-a" relationship bad for classes containing multiple instances of the same inner class? Is there a better way to model the classes in this case?

EDIT:

A few clarifications to the problem based on the comments on this question and the answers. The members x and y of Point cannot be made public but can only be manipulated through the public function setCoord.

Even if I replace Point *p1, *p2 with Point p1, p2, someone can just do line.p1 = Point() and change p1 and p2. Need to prevent this as well.

**−1** Not the real code. E.g. a class definition with no semicolon will cause compilation errors. — Cheers and hth. - Alf, Apr 21 '16 at 12:19
Btw, making them public kind of destroys the point of OOP and `class`, just use `struct` instead. — , Apr 21 '16 at 12:20
In general "has-a" relationship is just fine and [mostly prefered](http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance) — rozina, Apr 21 '16 at 12:21
@Cheersandhth.-Alf Why does example code have to compile? the code is used to illustrate a problem, not to compile. — rozina, Apr 21 '16 at 12:22
@rozina because a code that does not compile, illustrates a problem that prevents the program from compiling, not the problem that the question is about (unless the question is about the specific problem that prevents the compilation). You cannot see the difference between observed and expected behaviour, if you can't observe the behaviour. — eerorika, Apr 21 '16 at 12:47
@rozina because people illustrate their problem with not-the-code, people answer based on the assumption that the code is illustrating the problem, and the OP says "oh that is not what I meant". It not compiling is a separate issue, in that it prevents others from copy-pasting and playing easily; but not-the-code is bad enough. Often not-the-code can be so far from the code that there is ambiguity in what it means. So we ask SO askers to generate code that actually illustrates their issue, not pseudo-code where readers must read the OP's tea leaves. — Yakk - Adam Nevraumont, Apr 21 '16 at 12:57
@Yakk I see what you mean. Although I don't think not ending a class with a semicolon really warrants a -1 :) — rozina, Apr 21 '16 at 13:19
@roz the -1 is for not posting real (simplified, but real) code: the `;` is evidence that it is not the real code. — Yakk - Adam Nevraumont, Apr 21 '16 at 14:31
@rozina: The lack of the semicolon is proof that this isn't copy/pasted from your text editor in which you're composing your [MCVE]. That leaves us with the crucial question: what _other_ "accidental mistakes" did you make when transposing the code here? Suddenly we can't trust anything in the question. — Lightness Races in Orbit, Apr 21 '16 at 14:54
@LightnessRacesinOrbit I know, but we usually use struct when we want data to be public, not class. — , Apr 22 '16 at 08:39

Rotem · Answer 1 · 2016-04-21T12:47:07.577

3

First off, A line segment with two points is a bad example for a class with pointers. It's hard to think of a case in which you would not prefer the Points as value members rather than pointers.

In the more general sense, to signify to calling code that it should not change pointers, make them private and create getter methods which return references to the data. Add constness where applicable.

class ResourceOwner
{
private:
    Resource *r;
public:
    inline Resource& GetResource() { return *r; }

    // implement rule of 5
};

edited Apr 21 '16 at 12:47

answered Apr 21 '16 at 12:20

Rotem

21,452
6
62
109

4

Fyi, `inline` is redundant when method is defined inside of a class. – HolyBlackCat Apr 21 '16 at 12:26

score 3 · Answer 2 · answered Apr 21 '16 at 12:52

3

Is there a better way to model the classes in this case?

Since Line owns the Points, it should simply have two values:

class Line
{
public:
    Point p1, p2;
};

Now the Point invariants guarantee that a line is actually made of two points, and no one can put in e.g. an null pointer. Nothing else needs to be enforced, and replacing a Point with any other Point still makes a valid Line.

While we're at it, Point is also unnecessary complicated. Here's a much better version:

class Point
{
public:
    int x, y;
};

Using struct here instead makes the code even clearer.

answered Apr 21 '16 at 12:52

Bartek Banachewicz

38,596
7
91
135

This is just a simplified version of the problem. The member variables of Point cannot be made public and have to be manipulated using the methods. Also, I need that p1 and p2 cannot be replaced by a Point object other than the ones created in the Line's constructor, but be able to call its methods. Thanks! – Shaunak Gupte Apr 21 '16 at 13:36
2

@ShaunakGupte When you post a simplified version of the problem, you'll get a simplified version of the solution. – Bartek Banachewicz Apr 21 '16 at 13:37
Sorry for that. I will add more details to the question. – Shaunak Gupte Apr 21 '16 at 13:38
While I agree that in by far most cases two member points make more sense than pointers, I could think of a situation where points are moved dynamically (e.g. simulating atoms in a H2 molecule) and a line with pointers is used to define the bond structure...just a thought. – philkark Apr 21 '16 at 14:24
@phil13131 If those points are supposed to exist elsewhere, the `Line`'s constructor won't create them. – Bartek Banachewicz Apr 21 '16 at 14:54
@BartekBanachewicz That is of course true. I was just suggesting a situation where a class being a "line" could contain pointers to points rather than members. I did not refer to how they are created in that case. – philkark Apr 21 '16 at 17:08

Disadvantages of "has-a" relationship, C++

2 Answers2