C++ should I use pointer or reference?

Question

I would like to have an insight about whenever I should be using references or pointers.

Let's take the example of a Polygon class using a Rectangle class for its internal bounding box.

Polygon.h

class Polygon {

private:
    std::list<Point> _points;
    Rectangle _boundingBox;

public:
    Polygon(const std::list<Point> &);

public:
    const std::list<Point> &getPoints() const;
    const Rectangle &getBoundingBox() const;

private:
    void setBoundingBox();

};

Polygon.cpp

#include <iostream>
#include "Polygon.h"

Polygon::Polygon(const std::list<Point> &points)
{
    if (points.size() < polygon::MIN_SIDE + 1) {
        throw std::invalid_argument("A polygon is composed of at least 3 sides.");
    }

    if (points.front() != points.back()) {
        throw std::invalid_argument("A polygon must be closed therefore the first point must be equal to the last one.");
    }

    std::list<Point>::const_iterator it;
    for (it = ++points.begin(); it != points.end(); ++it) {
        this->_points.push_back(*it);
    }
    this->setBoundingBox();
}

void Polygon::translate(const std::array<float, 2> &vector)
{
    std::list<Point>::iterator it;
    for (it = this->_points.begin(); it != this->_points.end(); ++it) {
        (*it).setX((*it).getX() + vector[0]);
        (*it).setY((*it).getY() + vector[1]);
    }
    Point topLeft = this->_boundingBox->getTopLeft();
    Point bottomRight = this->_boundingBox->getBottomRight();

    topLeft.setX(topLeft.getX() + vector[0]);
    topLeft.setY(topLeft.getY() + vector[1]);
    bottomRight.setX(bottomRight.getX() + vector[0]);
    bottomRight.setY(bottomRight.getY() + vector[1]);
}

const std::list<Point> &Polygon::getPoints() const
{
    return this->_points;
}

const Rectangle &Polygon::getBoundingBox() const
{
    return this->_boundingBox;
}

void Polygon::setBoundingBox()
{
    float xMin = this->_points.front().getX();
    float xMax = this->_points.front().getX();
    float yMin = this->_points.front().getY();
    float yMax = this->_points.front().getY();

    std::list<Point>::const_iterator it;
    for (it = this->_points.begin(); it != this->_points.end(); ++it)
    {
        Point point = *it;
        if (point.getX() < xMin) {
            xMin = point.getX();
        }
        if (point.getX() > xMax) {
            xMax = point.getX();
        }
        if (point.getY() < yMin) {
            yMin = point.getY();
        }
        if (point.getY() > yMax) {
            yMax = point.getY();
        }
    }
    this->_boundingBox = new Rectangle(Point(xMin, yMin), Point(xMax, yMax));
}

std::ostream &operator<<(std::ostream &out, const Polygon &polygon)
{
    std::list<Point>::const_iterator it;

    for (it = polygon.getPoints().begin(); it != polygon.getPoints().end(); ++it) {
        out << (*it);
        if (it != polygon.getPoints().end()) {
            out << " ";
        }
    }
    return out;
}

Rectangle.h

#pragma once

#include <stdexcept>

#include "Point.h"

class Rectangle {

private:
    Point _topLeft;
    Point _bottomRight;

public:
    Rectangle(const Point &, const Point &);

public:
    const Point &getTopLeft() const;
    const Point &getBottomRight() const;
    float getWidth() const;
    float getHeight() const;
};

Rectangle.cpp

#include "Rectangle.h"

Rectangle::Rectangle(const Point &topLeft, const Point &bottomRight)
{
    if (topLeft.getX() > bottomRight.getX() || topLeft.getY() > bottomRight.getY()) {
        throw std::invalid_argument("You must specify valid top-left/bottom-right points");
    }
    this->_topLeft = topLeft;
    this->_bottomRight = bottomRight;
}

const Point &Rectangle::getTopLeft() const
{
    return this->_topLeft;
}

const Point &Rectangle::getBottomRight() const
{
    return this->_bottomRight;
}

float Rectangle::getWidth() const
{
    return this->_bottomRight.getX() - this->_topLeft.getX();
}

float Rectangle::getHeight() const
{
    return this->_bottomRight.getY() - this->_topLeft.getY();
}

Point.h

#pragma once

#include <ostream>
#include <cmath>

class Point {

private:
    float _x;
    float _y;

public:
    Point(float = 0, float = 0);

public:
    float distance(const Point &);

public:
    float getX() const;
    float getY() const;
    void setX(float);
    void setY(float);
};

std::ostream &operator<<(std::ostream &, const Point &);
bool operator==(const Point &, const Point &);
bool operator!=(const Point &, const Point &);

Point.cpp

#include "Point.h"

Point::Point(float x, float y)
{
    this->_x = x;
    this->_y = y;
}

float Point::distance(const Point &other)
{
    return std::sqrt(std::pow(this->_x - other.getX(), 2) + std::pow(this->_y - other.getY(), 2));
}

float Point::getX() const
{
    return this->_x;
}

float Point::getY() const
{
    return this->_y;
}

void Point::setX(float x)
{
    this->_x = x;
}

void Point::setY(float y)
{
    this->_y = y;
}

std::ostream &operator<<(std::ostream &out, const Point &point)
{
    out << "(" << point.getX() << ", " << point.getY() << ")";
    return out;
}

bool operator==(const Point &p1, const Point &p2)
{
    return p1.getX() == p2.getX() && p1.getY() == p2.getY();
}

bool operator!=(const Point &p1, const Point &p2)
{
    return p1.getX() != p2.getX() || p1.getY() != p2.getY();
}

A lot of questions come with this snippet of code.

• This does not compile because obviously whenever we try to create a Polygon, it ends up trying to create a Rectangle with a default constructor which does not exist.
• I can't use initializer list because obviously the bounding box depends on some computed values from my list of points.
• I could create a default constructor creating two Point(0, 0) by default for the Rectangle but this does not make much sense.
• I could use pointers but then I feel this is not the nicest solution as I tend to think this is mostly used for polymorphism in C++ we should prefer reference whenever possible.

How should I then proceed ?

I feel I am missing something out about C++ and could learn a lot from this.

Answer 1

I think your main question is about how to deal with the problem of needing to initialize both std::list<Point> _points; and Rectangle _boundingBox;, while also doing some validation of _points.

The simplest solution is to just give Rectangle a default constructor (or pass two default Points as initializer). Then once you have validated the points argument in the constructor, you calculate the Rectangle based on the points.

---

A slightly more complicated alternative is to allow the validation function to be invoked from the ctor-initializer list, e.g.:

Polygon::Polygon(std::list<Point> points)
    : _points( validate_point_list(points), std::move(points) ), _boundingBox( calculateBoundingBox(_points) ) 
{
}

where you have functions (which could be free functions):

void validate_point_list(std::list<Point> &points)
{
    if (points.size() < polygon::MIN_SIDE + 1)
        throw std::invalid_argument("A polygon is composed of at least 3 sides.");

    if (points.front() != points.back())
        throw std::invalid_argument("A polygon must be closed therefore the first point must be equal to the last one.");

// caller must pass in same first and last point, but we only store one of the two
    points.erase( points.begin() );
}

and

Rectangle calculateBoundingBox(std::list<Point> const &_points)
{
    // whatever logic you have in setBoundingBox, except return the answer
}

Note that the loop in your Polygon constructor is unnecessarily complicated. You could have just written _points = points; and then erased the extra point (which is O(1) for lists).

---

Note that I have passed by value and then used std::move. The reason is that if the argument given is a rvalue then it can just be moved right on through into where it's being stored; whereas with the const & version, a copy is stored and then the original is destructed.

I would use const & a lot less than you did. Small objects , such as Point and Rectangle, don't suffer a performance penalty from pass-by-value (and might even be more efficient that way). And as mentioned in the previous paragraph; if your function accepts a parameter and it is going to take a copy of that parameter, it's better to pass by value .

Passing by reference is best only when you are using but not storing the values passed. For example, calculateBoundingBox.

---

Finally, once you get this working, you might want to think about having the Polygon constructor accept an iterator pair of points range, and/or a std::initializer_list.