2

I have following problem:

Let's assume I have a Vector with 100 Objects, therefore it would be bad to create a copy.

class MyClass
{
 private:
 vector<MyObject> mVector;
};

//return by reference
vector<object>& MyClass::GetVector1()
{
 return mVector;
}

vector<object>* MyClass::GetVector2()
{
 return &mVector;
};

return by reference is fast and generates no copy. The vector should be still available in MyClass, therefore i don't want to move it. But what would be the best way to use this GetVector() method?:

class MyOtherClass
{
 private:
    vector<MyObject>* myVector;
    vector<MyObject>& myVector2;
    vector<MyObject>  myVector3;

    MyClass m;
};

myVector2 = m.GetVector1(); //only works in an initializer list!

But what if I can't initialize the vector when the object is created? Let's assume I have a OnInit() method which will be called when it's needed:

void MyOtherClass::OnInit()
{
 myVector = m.GetVector2(); //no copy using return by reference
 myVector = &m.GetVector1(); //no copy returning a pointer
 myVector3 = m.GetVector1(); //copy
}

Lots of people told me using pointers like myVector is very bad, but why????? Should I use smart_pointers instead? (Please give me a example, how to use them with the given code above)

Or are there better ways to get fast performance?.

Edit:

for the answer check my selected answer for this below.

additionally I want to add move semantics.

But this heavily depends on the purpose of the code, sometimes you really want to copy, you won't be able to avoid it. If you know the object last longer than the code tracking it, pointers are fine as long as you want changes to apply as well, otherwise use const ref.

If you don't need the object you return anymore and it might get destroyed, you would use return by value, which is ok since the compiler might optimize it by moving, but you can explicitly make use of movesemantics and move the vector out of the class, since the vector elements are dynamically allocated and the container is moveable. check this link: Is returning by rvalue reference more efficient?

jeromintus
  • 367
  • 1
  • 5
  • 14
  • 1
    Don't look at smart pointers as pointers, instead look at them in terms of *ownership* of the contained pointer. – Some programmer dude Nov 26 '14 at 10:03
  • Why don't you ask the people who told you that to elaborate? – juanchopanza Nov 26 '14 at 10:11
  • so i have to use a pointer or reference, but aren't raw pointers bad? – jeromintus Nov 26 '14 at 10:36
  • 4
    @slei: They're bad if you try to use them to manage ownership, or to point to things that are likely to get destroyed. They're not bad if you simply want to point to something that's going to outlive the pointer. – Mike Seymour Nov 26 '14 at 10:41
  • in other forums someone told me, such pointer can lead to unexpected bugs and this should be a better solution: (example using no container) `std::shared_ptr _test; const std::shared_ptr& GetTest() const { return _test; }` – jeromintus Nov 26 '14 at 11:03
  • @slei "A shared_ptr represents shared ownership but shared ownership isn't my ideal: It is better if an object has a definite owner and a definite, predictable lifespan. " - Bjarne Stroustrup. – mip Nov 26 '14 at 11:29
  • what would be a definite owner and definite,predictible lifespan? :P – jeromintus Nov 26 '14 at 12:33
  • @slei owner can do whatever he wants with owned object, including its destruction. Definite owner is in opposite to shared ownership, where many objects are allowed destroy their shared asset. `shared_ptr` addresses this issue. If ownership is shared then it is hard to predict object lifespan, because you don't know which object will destroy it. – mip Nov 26 '14 at 14:27
  • I have not that much experience with ownership, because I've nearly never used smart pointers, but thanks :) – jeromintus Nov 26 '14 at 14:48
  • If you give other code access to your internals like that, you really ought to be returning `const std::vector&` (or `const std::vector*`), unless you're completely prepared for the container to be modified behind your back. – Toby Speight Mar 14 '18 at 15:05

2 Answers2

3

Public object is fine

At that point just have a public object. You don't need that getter:

struct X {
    std::vector<Y> vec;

    X(..) : vec(..) { .. }
};

Then you can simply use it as:

X x(..);
x.vec.some_member_function();

auto& z = x.vec;
z.vec.some_member_function();

If you really have to

If you really need to use the member function version, then use return by reference, which is a little bit more idiomatic, even though it's basically the same thing as return by pointer performance wise:

vector<Y>& X::GetVector() {
    return mVector;
}

and then use it as:

X x(..);
x.GetVector().some_member_function();

auto& z = x.GetVector();
z.some_member_function();

Smart pointers are not for this

Smart pointers solve the problem of handling resources and describe ownership. In this context they wouldn't make much sense given that std::vector already owns and handles its own resources internally.

Shoe
  • 74,840
  • 36
  • 166
  • 272
  • but what if the vector should be available in the whole class? – jeromintus Nov 26 '14 at 10:10
  • 2
    @slei The vector is available in the whole class in both cases. What do you mean exactly? – Shoe Nov 26 '14 at 10:11
  • to make z available in the whole class I would put in the header: and then initialize it in the .cpp, but using a reference variable only allows to initialize it with a initializer list at the constructor – jeromintus Nov 26 '14 at 10:14
  • The second method with a getter is more than just idiomatic, it provides a very useful way to track accesses to this variable in the future. You can stick a breakpoint on a Get method, or do some quick state verification, you can't easily make that change to a public member later on. – foips Nov 26 '14 at 10:17
  • @slei In all the examples `z` is simply a reference to the vector that you store somewhere outside of the class. `x` is also an instance of class `X`. In my examples, you can access the vector within the class by using the identifier `vec`. – Shoe Nov 26 '14 at 10:23
  • what if I want to initialize the vector in a particular method, but the vector should be still available for the whole class? – jeromintus Nov 26 '14 at 10:35
  • @slei The only place you can initialize the vector at construction is in the initializer list of the constructor of `X`. Also in any case the vector *is* available in the whole class. Where do you think it's not available within the class? – Shoe Nov 26 '14 at 10:37
  • Thats my problem, in my case I would like to use GetVector() in an Particluar method, because the constructor is called befor the vector is created, but the vector should still be available in the whole class and not just be a local variable – jeromintus Nov 26 '14 at 10:41
  • @slei explain this problem more verbosely. Maybe you need base from member idiom? – mip Nov 26 '14 at 11:06
  • @slei No, the constructor is called **after** the vector is created, so you can use it anywhere. See [here](http://coliru.stacked-crooked.com/a/e3860f80c8a8469e) for an example. – Shoe Nov 26 '14 at 12:17
  • i mean, vector which will be returned is created after the constructor where i call GetVector() is created, therefore i can't use the initializer list. I just would like to see a good way with fast performance without using a reference variabel/whatever, which must be initialized in the initializer list – jeromintus Nov 26 '14 at 12:36
  • @slei please start another question and provide an example. From what you say the only problem is that vector object is created too late. But why can't you create this vector object before you initialize a reference? Also if you initialize reference for performance reasons, then it doesn't make any sense - just use `GetVector()` method, it will be inlined. – mip Nov 26 '14 at 14:06
  • @slei Can you make a code example (use ideone or pastebin for the code) of the problem? – Shoe Nov 26 '14 at 15:48
  • coming back to this with more exp. this advice is still true, but I'd still recommend using a Getter, having one entry point heavily helps tracking down bugs. especially if the class is widly used, it isn't fun at all not knowing who is constantly changing your public variable, even with an IDE data breakpoint ._. – jeromintus Mar 14 '18 at 12:23
  • You forgot the third alternative: Refactor so that you don't need `myVector` to be public. Of course, that's not always possible, but definitely an alternative worth exploring before going down the no-encapsulation-whatsoever path. – cmaster - reinstate monica Mar 14 '18 at 12:38
0

If myVector can be initialized by initializer list then do it! Otherwise you have to use pointer.

Pointers are not bad, they are just less restrictive than references. If nullptr/0 value is accepted for myVector then you can use pointer. When you use reference it tells you that it will not be null (or at least shouldn't be) and you don't need to check if myVector == 0 when you want to use it.

I apply same convention to return values. For example to me declaration like this

vector<object>* MyClass::GetVector2()

means that function may return nullptr/0, while

vector<object> & MyClass::GetVector2()

can not.

To use pointer or reference safely you have to make sure that MyClass object will outlive MyOtherClass object. In your example it is satisfied, since MyClass object is a member of MyOtherClass.

Now about your design. Why initialize reference if you can accessMyClass::mVector by GetVector() function? Also, do you really need such access? It would be better if MyClass would maintain everything related to mVector.

mip
  • 8,355
  • 6
  • 53
  • 72