Good point. In those cases, the copy constructor should have been deleted to prevent the copy build up. For objects when copy constructor is deleted *and* compiler doesn't perform RVO, I think we'd likely need to add a move constructor.
Thanks, David On Wed, Sep 6, 2017 at 2:01 PM, Jacob Barrett <jbarr...@pivotal.io> wrote: > 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 > > > > > >
