Bare object sounds nice to me. The caller has the option at that point to copy to heap into smart pointer or keep on stack.
Mixed with the fluent/builder conversation in another thread this would simplify to: auto cache = CacheFactory().setSomething().create(); Which I think looks pretty darn readable. If they want to stash that cache in the heap and pass it around to other callers then: auto cache = std::shared_ptr<Cache>(CacheFactory().setSomething().create()); Which isn't as nice to read but gets the job done. I think with many of these calls they are not frequent calls to if any of them resulted in copies it wouldn't be the end of the world. What we need to make sure doesn't happen though is that a copy builds up more of the underlying resources. I would not want a copy of Cache to returned by CacheFactory to result in another set of connections to the servers. Have you looked closely at what copy would do in the case of each of the objects we are looking at on the public API? Assuming no optimization and a copy is performed then are there resources be re-allocated that we don't want re-alloacted. If that is the case then we need to consider alternatives like ref, ptr or pimpl. -Jake On Wed, Sep 6, 2017 at 12:53 PM Ernest Burghardt <eburgha...@pivotal.io> wrote: > std::unique_ptr looks like a good option. > > then again if the typical usage is like so: > > cachePtr = CacheFactory::createCacheFactory(pp)->create(); > > it seems simple enough to just return the bare object > > EB > > On Wed, Sep 6, 2017 at 1:27 PM, David Kimura <dkim...@pivotal.io> wrote: > > > What type should C++ object creation functions return (e.g. pointer, > smart > > pointer, reference, etc.)? Several of our C++ API's return shared > > pointers. For example in the following function signature [1]: > > > > std::shared_ptr<CacheFactory> CacheFactory::createCacheFactory(...); > > > > Here the only case I can see for shared pointer is to indicate ownership > of > > CacheFactory. Ideally this should probably be std::unique_ptr because > > callee shouldn't share ownership. However, I don't see the point of > using > > a pointer at all.. I suggest we return the bare object, like the > following > > signature: > > > > CacheFactory CacheFactory::createCacheFactory(...); > > > > In C++03, this would have been a performance hit because we'd end up with > > an added call to the copy constructor. In C++11, std::unique_ptr gives > > std::move for free and thus avoids copy-constructor call. However, most > > modern C++11 compilers already perform copy-elision here. In fact, C++17 > > standard dictates that compilers must perform RVO here. Therefore it > > doesn't seem to me that std::shared_ptr or std::unique_ptr buys us much > in > > this situation. > > > > Thoughts? > > > > Thanks, > > David > > > > > > [1] https://github.com/apache/geode-native/blob/develop/ > > cppcache/include/geode/CacheFactory.hpp#L54 > > >
