To me the question of a no-op region comes down to whether customers would need
to do logic based on whether a region has a parent. If they do, perhaps a bool
hasParent() along with getParent() returning a no-op region would be an elegant
solution. If they do not, then shared_ptr is probably sufficient.
Sarge
> On 10 Oct, 2017, at 12:45, David Kimura <dkim...@pivotal.io> wrote:
>
> 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?
>>>>
>>>
>>
