Law of demeter or return the whole vector

Question

Which one is better:

public:
  const vector<int> & GetPointsVector();
private:
  vector<int> PointsVector;

Or:

public:
  int GetCurrentPoint();
  void MoveToFirstPoint();
  void MoveToNextPoint();
  bool IsAtLastPoint();
  size_t GetNumberOfPoints();
private:
  vector<int> PointsVector;

Answer 1

Neither. Make the function which needs such intimate knowledge of the class's internals into a member function.

Answer 2

In my opinion, the first alternative (strictly with the const keyword) is better, since then you can use standard interfaces to access the members of the vector (such as iterators, etc.). The second option (i.e. dedicated access methods) is required only when you have no other option to protect the data from accidental modifications.

Answer 3

Both are not. Better return the begin() and end() iterators, or still better a boost::range for the iterator.

private:
        typedef std::vector<int>            PointsContainer;
public: 
        typedef boost::iterator_range<PointsContainer::const_iterator> PointsRange;
        PointsRange getPointsRange() const {
            return boost::make_iterator_range(pointsContainer_.begin(), pointsContainer_.end()); 
        }

The advantage is that the traversal logic is hidden within the range/iterator

While using, one alternative is to do:

int p;
foreach(p, obj.getPointsRange()) {
   //...
}

otherwise

C::PointsRange r = obj.getPointsRange();
for(C::PointsRange::iterator i = r.begin(); i != r.end(); ++i) {
      int p = *i;
      //...
}

Answer 4

I prefer incapsulate internal data presentation and use forEachSomething template public function. Or if you have boost you can avoid templated forEachSomething and put implementation to the .cpp file.

class Class
{
public:
    Class()
    {
        points_.push_back( 1 );
        points_.push_back( 5 );
        points_.push_back( 7 );
    }

    template <typename TFunction>
    void forEachPoint( TFunction function )
    {
        std::for_each( points_.begin(), points_.end(),
                       function );
    }

    void forEachPoint2( boost::function< void (int ) > function )
    {
        std::for_each( points_.begin(), points_.end(),
                       function );
    }
private:
    std::vector<int> points_;
};

void print( int a )
{
    std::cout << a << std::endl;
}

int main()
{
    Class object;
    object.forEachPoint( print );
}

Answer 5

There is no right answer. The answer will vary with context, of which, at present we have very little.

It depends on the clients of your class. In most situations you would not want a casual inspector to change the object's state, so it is better to a certain degree to return a reference to a const object. However, in such a case, I'd also make the function a const i.e.

 /* a design const - this accessor does not change the state */
 const vector<int> & GetPointsVector() const;

This is also efficient since you are not passing around heavy objects as return values (it is a different question that most compilers do a RVO these days).

If, your client needs to use the vector in algorithms, yes, you'd better provide iterators. But two pairs i.e.

typedef vector<int>::iterator _MyItr;
_MyItr begin() const;
_MyItr begin();

_MyItr end() const;
_MyItr end();

This would be in keeping with the way the STL is designed. But, be careful in specifying what sort of iterator the client can expect (for example: if the class specification says that the iterators returned are RandomIterators, it sets a certain amount of expectation on your implementation).