4
\$\begingroup\$

I am just beginning to dabble in C++. Would you implement this as a class like I did, or a struct? What are the pros and cons?

class Point
{
private:
    double _x;
    double _y;

public:
    Point();
    Point(double x, double y);
        ~Point();
        Point(Point& other);
        double GetX();
        void SetX(double x);
        double GetY();
        void SetY(double y);
};



Point::Point() : _x(0.0), _y(0.0)
{
}

Point::Point(double x, double y) : _x(x), _y(y)
{
}

Point::Point(Point& other) : _x(other._x), _y(other._y)
{
}

Point::~Point()
{
}

double Point::GetX()
{
    return _x;
}

void Point::SetX(double x)
{
    _x = x;
}

double Point::GetY()
{
    return _y;
}

void Point::SetY(double y)
{
    _y = y;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ This should give you an idea of the differences between the two: When should you use a class vs a struct in C++? \$\endgroup\$
    – Jamal
    Jun 10 '13 at 6:22
  • \$\begingroup\$ I am not sure whether I would use a class or a struct, but I would declare x and y const so that my objects were immutable, and all my multi-threading issues go away. \$\endgroup\$ Jun 12 '13 at 3:14
5
\$\begingroup\$

As they stand right now, your destructor and copy constructor accomplish nothing that the compiler won't do just as well on its own. You're probably better off eliminating them.

You might also consider merging your two constructors:

Point::Point() : _x(0.0), _y(0.0)
{
}

Point::Point(double x, double y) : _x(x), _y(y)
{
}

...into a single constructor with default arguments:

explicit Point::Point(double x = 0, double y = 0) : _x(x), _y(y) {}

Note the explicit though -- without it, the single constructor can support implicit conversions, so if (for example) you had something like:

void f(Point const &a);

...the compiler wouldn't stop you from a mistake like f(1), because it sees the constructor that allows it to do f(Point(1, 0.0)). Even with the explicit, using two separate constructors is arguably a bit safer. Just for example, if you were creating a temporary point object in some deeply nested set of function calls:

f(g(h(Point(a,b))));

and accidentally typed that as:

f(g(h(Point(a),b)))

...with two separate constructors, the compiler would diagnose the problem fairly directly, but with one constructor and default arguments, it might not be quite so direct (especially if h could also take a second argument). Frankly, however, I think that's a remote enough possibility I'd probably live with it.

Then we get to the age-old question of whether you gain anything from using get/set pairs for your x and y. For a point, there's a better theoretical argument than with most types that you might eventually want to change representation and use polar coordinates instead of Cartesian coordinates. While theoretically sound, my experience is that this basically just doesn't happen. In code where it makes sense to store polar coordinates, you're generally working with polar coordinates externally as well, so your Cartesian-based interface is wrong anyway.

At the same time, using a getter means that (for example) to translate a point to the right by four units, you end up with something ugly like my_point.set_x(my_point.get_x() + 4); instead of just my_point.x += 4;.

Bottom line: using a getter/setter (accessor/mutator, if you prefer) for this job does a great deal more harm than good. The benefit runs right on the ragged edge between "purely theoretical" and "completely imaginary" while the harm happens essentially all the time in every piece of real code that uses such a class.

Based on what it actually accomplishes, you'd probably be better off with something much simpler, like:

struct Point {
    double x;
    double y;

    explicit Point(double x = 0.0, double y=0.0) : x(x), y(y) {}
};

That assures the members are initialized, and the compiler-synthesized copy constructor, destructor, copy assignment, move constructor, move assignment will all work fine. Direct access to the x and y members will clean up client code considerably.

\$\endgroup\$
5
\$\begingroup\$

If you are using point as a property bag (like this).

Then just make it a struct (leave the members public). If you think there is a slight possibility of modifying the implementation then use get/set (ers) to access the memebrs but this class is so simple it seems over kill.

If this is part of a school class then your professor is going to say use member accessors so that the internal implementation can change and the internal implementation can change without the class interface being changed.

Though true in this simplistic case its just not going to happen.

Though I would add constructors to make sure the members are correctly initialized.

PS. Try not to use a leading underscore on identifiers. Though in this case it is not a problem the rules are non trivial and not well known so best avoided. see https://stackoverflow.com/a/228797/14065

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.