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 <jbarr...@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 <dkim...@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 <jbarr...@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?
> > >
> >
>
