Re: [Discuss] What type should C++ object creation functions return?

[David Kimura]

Sure.  If we made these changes RegionFactory::create would change to:

Region RegionFactory::create(const char*);

Since Region has a deleted copy constructor, we would have to implement a
move constructor and a move assignment operator.

Region(const Region&& other);
Region& operator=(const Region&& other);

Thus I think a call like following should not invoke copy constructor even
if compiler doesn't support copy-elision:

auto region = RegionFactory().create(...);

Thanks,
David

On Wed, Sep 6, 2017 at 2:31 PM, Jacob Barrett  wrote:

> Great, can you provide some examples of what some of the APIs would look
> like if we made these changes?
>
> On Wed, Sep 6, 2017 at 2:29 PM David Kimura  wrote:
>
> > 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 
> 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(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 
> > 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::
> > > 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
> > > > >
> > > >
> > >
> >
>

Re: [Discuss] What type should C++ object creation functions return?

[Jacob Barrett]

Great, can you provide some examples of what some of the APIs would look
like if we made these changes?

On Wed, Sep 6, 2017 at 2:29 PM David Kimura  wrote:

> 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  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(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 
> > 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 
> 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::
> > 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
> > > >
> > >
> >
>

Re: [Discuss] What type should C++ object creation functions return?

[David Kimura]

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  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(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 
> 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  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::
> 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
> > >
> >
>

Re: [Discuss] What type should C++ object creation functions return?

[Jacob Barrett]

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(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 
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  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::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
> >
>

Re: [Discuss] What type should C++ object creation functions return?

[Ernest Burghardt]

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  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::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
>

Re: [Discuss] What type should C++ object creation functions return?

[Michael William Dodge]

I like just returning the object as that removes the possibility of a null 
pointer.

Sarge

> On 6 Sep, 2017, at 12:27, David Kimura  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::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