Mailing-List: contact dev-help@geode.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.apache.org
MIME-Version: 1.0
In-Reply-To: <CAA1wVxq=YMMhpDa+nw4VT30sGV9Jf4Ca+dT1U=YxX91dUnrLjw@mail.gmail.com>
References: <CAA1wVxogTpBsESx6g=Eema1s9sd7UQGprCgNH8mbksQOoTht5w@mail.gmail.com>
 <CAD=mLVobGFx0hNHs_YnC7ZNzwF+xU--3DosV38jMOnpCpKwD1g@mail.gmail.com> <CAA1wVxq=YMMhpDa+nw4VT30sGV9Jf4Ca+dT1U=YxX91dUnrLjw@mail.gmail.com>
From: David Kimura <dkimura@pivotal.io>
Date: Tue, 10 Oct 2017 12:45:18 -0700
Message-ID: <CAD=mLVq77EaQTHS5X_bnmxWvYa6aYMmytsN4_60TXb_xAtbSow@mail.gmail.com>
Subject: Re: [DISCUSS] C++ Returning null values
To: dev@geode.apache.org
Content-Type: multipart/alternative; boundary="94eb2c1cd2e60c11e2055b368cfa"
archived-at: Tue, 10 Oct 2017 19:45:25 -0000

--94eb2c1cd2e60c11e2055b368cfa
Content-Type: text/plain; charset="UTF-8"

Yes, but I wouldn't expect to ever need to do a type check. Admittedly, I'm
not sure what one does with parent region, but if we no-op, return sensible
empty values, or throw when appropriate then maybe we don't care?

I would expect it to be used something like this:

auto parent = region.getParent();

// if region didn't have parent region, this could always return false.
// else if it did have a parent region it would behave as expected.
parent.containsKey("a_key");


If this works, I like that we don't have to special case for calling
getParent on root region and we could return by value.

Thanks,
David

On Tue, Oct 10, 2017 at 12:29 PM, Jacob Barrett <jbarrett@pivotal.io> wrote:

> Are you thinking something like?
>
> class NoRegion : public Region {...};
>
> auto parent = region.getParent();
> if (NoRegion == parent) {
>   // no parent region
> }
>
>
> On Tue, Oct 10, 2017 at 11:08 AM David Kimura <dkimura@pivotal.io> wrote:
>
> > I'm not sure if this is the same as the sentinel value you mentioned, but
> > what about introducing a no-op region type and returning that?  I'm
> > thinking a null object pattern which would no-op and then nobody should
> > need to check if nullptr.
> >
> > Thanks,
> > David
> >
> > On Tue, Oct 10, 2017 at 10:27 AM, Jacob Barrett <jbarrett@pivotal.io>
> > wrote:
> >
> > > Looking at a class like Region (Region.cpp) there are calls to get the
> > > parent region and sub regions, there are instances where a Region will
> > not
> > > have a parent or subs. The current API returns shared_ptr that may be
> > > nullptr or a Region.
> > >
> > > Since we are trying to make an effort towards values over pointers
> should
> > > be considered some changes here? Obviously a reference is out of the
> > > question because it can't be null. A value is really out of the
> question
> > > too since it can't be null and making a sentinel value is not a great
> > > solution. Raw pointers are fine since they can be nullptr but make
> > > ownership ambiguous. Using shared_ptr is good since it can be nullptr
> and
> > > solves the ownership problem. Another option is to embrace the
> > forthcoming
> > > std::optional available as boost::optional in C++11.
> > >
> > > I am leaning towards keeping it shared_ptr since using boost::optional
> > > would require users compile with boost. I don't think we should have
> > > anything on our API that is not ours of C++11. Requiring third party
> > > libraries to compile against our API doesn't fly right by me.
> > >
> > > Thoughts?
> > >
> >
>

--94eb2c1cd2e60c11e2055b368cfa--