2
votes
\$\begingroup\$

Unlike most(?) smart pointers, boost::intrusive_ptr has a non-explicit constructor. Given that, one could write

typedef boost::intrusive_ptr<Foo> FooPtr;

FooPtr MyFactory()
{
    return new Foo(a, b, c);
}

Or one could write

FooPtr MyFactory()
{
    return FooPtr(new Foo(a, b, c));
}

The latter is more consistent with how the code would have to be written if FooPtr were another kind of smart pointer, like shared_ptr or unique_ptr. The former is more concise. I personally prefer concise as long as it doesn't lead to trouble for readers and maintainers later. I'm wondering whether the former case is "too" concise?

\$\endgroup\$
1
2
votes
\$\begingroup\$

When in doubt, be explicit. While both of the functions given will work, the second expresses programmer intent more clearly. Adding a comment along the lines of "This will call intrusive_ptr's implicit constructor" to the first can help, though at that point it's more concise to call the constructor directly. Returning a raw pointer and relying on the implicit constructor for intrusive_ptr may also be viewed as a mistake introduced by changing the declaration of MyFactory() but forgetting its definition. Relying on implicit constructors may also make the code harder to debug and maintain because by default, the constructor for intrusive_ptr increments the retain count in Foo. Calling it explicitly gives you the benefit of being able to search for "FooPtr(" when trying to track down every place a Foo is retained. Also, if the Foo constructor initializes its retain count to 1, you may be introducing a memory leak by relying on the implicit constructor. Explicitly specifying FooPtr(new Foo(a, b, c), false) would prevent this.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.