0

I have a quick question regarding const-correctness for getters of vector of non-pointers.

Basically, I have this class with a non-const getter and a const getter for m_Vertices:

class Mesh
{
public:
    std::vector<Vertex>& GetVertices() { return m_Vertices; }

    const std::vector<Vertex>& GetVertices() const { return m_Vertices; }

private:
    std::vector<Vertex> m_Vertices;
}

The non-const getter makes sense; the getter is non-const, we return a non-const reference to a vector of non-const Vertex instances.

The const getter in the other hand doesn't makes sense to me. I'm questioning the const-correctness and whether I should just return a copy of the vector. The problem I see is that we return a const reference to a vector of non-const Vertex instances. So, the callee will have a const reference to m_Vertices, but will still be able to modify the Vertex instances, which feels wrong to me in terms of const-correctness.

In that case, should I simply return a copy of m_Vertices?

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
  • 2
    "The callee will still be able to modify the `Vertex` instances" Will it? `std::vector` doesn't work the way you think – Nathan Pierson Sep 02 '23 at 16:24
  • `const std::vector` does not allow to modify its elements so your assumption is incorrect. – Quimby Sep 02 '23 at 16:26
  • @NathanPierson it doesn't indeed. Thanks for the heads-up. – Anthony Blanchette-Potvin Sep 02 '23 at 16:33
  • I don't like this pattern, and try to avoid it in my own code. (Although *sometimes* it is the right pattern for the problem at hand.) It tightly couples the private member variable to the public API. If there were any invariants imposed upon `m_Vertices`, those invariants would be at the mercy of whatever manipulations the users of the `GetVertices` do to the member variable... why even bother to make it `private:` in the first place? Perhaps it has no preconditions/postconditions, and the class has no invariants...? – Eljay Sep 02 '23 at 17:02
  • @Eljay It's for a game engine, which have to render multiple meshes every frame. I believe returning a copy of the vector each time `GetVertices` is called would create a big overhead in big scenes. Also, I haven't identified pre-conditions/post-conditions nor invariants for the class. Truth be told, I'm not doing contract programming anyway. – Anthony Blanchette-Potvin Sep 02 '23 at 17:18

1 Answers1

1

First of all, understand that this is an opinion-based question. There are no right or wrong answers when you are designing a class regarding the usage of const.

However, that said, in this case your const declarations are reasonable. In particular your statement "the callee will have a const reference to m_Vertices, but will still be able to modify the Vertex instances" is not true -- because of the decisions that were made in the design of std::vector regarding const-ness.

There are three main ways someone might want to modify the vector and its elements. The callee may want to add/delete items to/from the vector, may want to change the value of a field of the elements, or may want to replace one of the elements. A reference to a const vector will make all three of those actions errors:

#include <iostream>
#include <vector>
#include <span>

struct point {
    double x;
    double y;
};

class vertices {
    std::vector<point> points_;
public:
    vertices(std::span<const point> pts) :
        points_{pts.begin(), pts.end()}
    {}

    const std::vector<point>& get() const {
        return points_;
    }

    std::vector<point>& get() {
        return points_;
    }
};

int main()
{
    vertices verts({{ {1.0,2.0}, {3.0,4.0}, {5.0,6.0} }});
    const auto& const_verts = verts;

    verts.get().push_back({ 7.0,8.0 }); // <= yes.
    //const_verts.get().push_back({ 7.0,8.0 }); <= no.

    verts.get().front().x += 1; // <= yes.
    //const_verts.front().x += 1; // <= also no.

    verts.get()[2] = { 0.0,0.0 }; // <= yes.
    //const_verts.get()[2] = { 0.0,0.0 }; <= no.
}
jwezorek
  • 8,592
  • 1
  • 29
  • 46