DutChen18
DutChen18

Reputation: 1173

Should a function return a "new" object

Should a function to return a pointer to memory allocated on the heap?

In other words, which of the following methods is more "correct"?

// Method #1
Object* getObject1() {
    return new Object();
}

// Method #2
std::shared_ptr<Object> getObject2() {
    return std::make_shared<Object>();
}

int main() {
    // Usage of method #1
    Object* o1 = getObject1();
    o1->doSomething();
    delete o1; // object has to be deleted by user

    // Usage of method #2
    std::shared_ptr<Object>& o2 getObject2(); // has to be kept in shared_ptr
    o2.get()->doSomething();
    // object is deleted when o2 destructs
}

I imagine the first method is probably faster, but the second method does not require the user to delete the object.

Upvotes: 5

Views: 15704

Answers (3)

Christian Hackl
Christian Hackl

Reputation: 27538

Neither option is really that great.

By default, the best thing you can do in C++ is this:

Object getObject1() {
    return Object();
}

If you really have to use dynamic allocation, then your first choice should be returning a std::unique_ptr by value:

std::unique_ptr<Object> getObject1() {
    return std::make_unique<Object>();
}

See "GotW #90 Solution: Factories" by Herb Sutter for a nice article on that subject. Among other things, Sutter says:

  • Returning unique_ptr expresses returning unique ownership, which is the norm for a pure “source” factory function.
  • unique_ptr can’t be beat in efficiency—moving one is about as cheap as moving/copying a raw pointer.

As for your specific questions:

I imagine the first method is probably faster,

std::shared_ptr involves locking in order to be thread-safe, so it may potentially decrease performance. However, it should not really matter. Dynamic allocation is typically always slower than the alternative, so you typically do not want to use it where micro performance matters (such as in a tight loop or in some part of the code which is executed many times per second). C++ discourages you from creating many small objects on the free store. So a function like this should not be able to be a bottleneck in the first place.

Upvotes: 4

Wagner Patriota
Wagner Patriota

Reputation: 5674

All of them are correctly. However, it's a good practice to create a "deleter" function for the first case...

why?

because you have an "interface" that's responsible allocating the object, then you should have another pair that does the deallocation, once the consumer of this function is using your interface, one doesn't need to know that a new is happening inside... it could be a malloc, for example, or anything else... so the pair of deleter function would implement the delete or free or whatever else you have to proper deallocate.

Upvotes: 1

David Schwartz
David Schwartz

Reputation: 182829

Of the two, the second would be preferred. Naked pointers should always be your last choice. But ideally, you would just return Object by value. Failing that, a unique_ptr would be better than a shared_ptr unless you really do need shared ownership.

I imagine the first method is probably faster.

That's probably true with shared_ptr. But with unique_ptr, the compiler can optimize. There's no benefit that pays you back for the risk of manual deletion.

Upvotes: 15

Related Questions