Cool. +1 value model. Thanks, David
On Tue, Sep 19, 2017 at 11:15 AM, Jacob Barrett <jbarr...@pivotal.io> wrote: > It isn’t. A Region should not outlive cache. > > > On Sep 19, 2017, at 10:54 AM, David Kimura <dkim...@pivotal.io> wrote: > > > > Oh, man. I hope I didn't just kill the original idea. Unless we are > > somehow using shared_from_this, it shouldn't be a divergence from > existing > > behavior? > > > > Thanks, > > David > > > >> On Tue, Sep 19, 2017 at 10:47 AM, Jacob Barrett <jbarr...@pivotal.io> > wrote: > >> > >> Region returned by this would no longer be valid. It’s “references” to > >> interns cache/region would be invalid. The behavior is undefined. > >> > >>> On Sep 19, 2017, at 10:24 AM, David Kimura <dkim...@pivotal.io> wrote: > >>> > >>> Err... I missed a return in example above. > >>> > >>> Region r = [](){ return CacheFactory::create().getRegion("myregion"); > } > >> (); > >>> > >>>> On Tue, Sep 19, 2017 at 10:19 AM, David Kimura <dkim...@pivotal.io> > >> wrote: > >>>> > >>>> I favor values, but we should probably be diligent. > >>>> > >>>> Do any of the objects returned from Cache depend on Cache to out-live > >> the > >>>> returned object? A quick scan suggested no, but we still have a > >>>> std::enable_shared_from_this<Cache>. Maybe dead code. In code > >> example, > >>>> if this happens (for any cache.get*), could we be in trouble or is > this > >>>> user error? > >>>> > >>>> Region r = [](){ CacheFactory::create().getRegion("myregion"); }(); > >>>> // Cache is out of scope. > >>>> // What is expected behavior? > >>>> r.put("key", "value"); > >>>> > >>>> Thanks, > >>>> David > >>>> > >>>> On Mon, Sep 18, 2017 at 7:44 PM, Jacob Barrett <jbarr...@pivotal.io> > >>>> wrote: > >>>> > >>>>> > >>>>> > >>>>>> On Sep 18, 2017, at 7:34 PM, Kirk Lund <kl...@apache.org> wrote: > >>>>>> > >>>>>> I would vote +1 for a more attractive, professional and > user-friendly > >>>>> API. > >>>>>> I'm not sure if there's a perf or memory-usage reason for using > >>>>>> "std::shared_ptr<?>" to types instead of returning values, but the > end > >>>>>> result does not look like a professional API to me. > >>>>> > >>>>> There really isn’t, especially if you look at what we did dirty > >>>>> CacheFactory::getCache by returning a value that can be moved into > the > >> heap > >>>>> and a shared point of one wants but not being forced into it. RVO > >> tricks > >>>>> can event make that move a noop. > >>>>> > >>>>> auto r = cache.getRegion(...); > >>>>> Where decltype(r) == Region > >>>>> and > >>>>> auto rp = std::make_shared<Region>(cache->getRegion()); > >>>>> Where decltype(rp) == shared_ptr<Region> > >>>>> > >>>>> Would both be valid. > >>>>> > >>>>> > >>>>> > >>>> > >> >