Question

I have a class which has a member of type std:vector

  private:
     std::vector<int> myVector;

I have created Get method to access myVector

1. const std::vector<int>   GetMyVector() const;
2. const void   GetMyVector(std::vector<int>& vec) const;

The implementations are as follows respectively:

1. const std::vector<int> MyClass::GetMyVector() const
   {
       return  myVector;
   }

2. const void MyClass::GetMyVector(std::vector<int>& vec) const
   {
       vec =  myVector;
   }

Which one of the two Get methods is better and why?

Was it helpful?

Solution

I'd prefer option 3:

const std::vector<int>& MyClass::GetMyVector() const
{
    return  myVector;
}

Your option 1 returned a copy of myVector. This returns a const (so read-only) reference to the class member.

OTHER TIPS

Why return the vector at all?

int MyClass::GetItem(const size_t index) const
{
    return myVector[index];
}

First of all you are exposing your implementation when you return a private member of a class from a member function, which usually is bad design. Take a look at @JoachimPileborg's solution for an example of how to avoid this.

If you want to return a copy then you should return by value.

If you want to return a reference to an object then return by reference. However, bear in mind that when the object is destructed you will end up with a dangling reference, e.g.

class Foo {
public:
    std::vector<int>& getVec() {
        return myVec;
    }
private:
    std::vector<int> myVec;
};

int main() {
    Foo* f = new Foo();
    std::vector<int>& myRef = f->getVec();
    delete f;

    std::cout << myRef.size(); // The demons come! Dangling reference!
}

Because of this, it is often the right thing to return a copy instead of a reference.

If you are returning an object by copy then there is no any sense to declare it as const. So instead of

const std::vector<int> MyClass::GetMyVector() const
{
    return  myVector;
}

I would write

std::vector<int> MyClass::GetMyVector() const
{
    return  myVector;
}

The second declaration is worst than the first one because it only confuses users. It is not clear whether the corresponding data member of the class is assigned to the parameter or this method makes some changes of the parameter without assigning the corresponding data member to the parameter.

So considering suggested variants by you I would choise declaration

std::vector<int> MyClass::GetMyVector() const
{
    return  myVector;
}

As a rule of thumb always try to return const reference of a class member . So use const std::vector<int> & MyClass::GetMyVector() const

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top