Re: [PATCH] Subscription

On 10/07/2010 01:54 AM, Amos Jeffries wrote:
> On 06/10/10 18:15, Alex Rousskov wrote:

>> You may also want to add something like:
>>
>> \todo Add a way for Subscriber to cancel the subscription and for
>> Publisher test for subscription cancellation?
>
> This is already provided by RefCount API inherited to
> Subscription::Pointer.

Yes, if we do not need to notify Publisher, then the underlying bits are
kind of already there, but the Publisher still needs a clean way to test
for cancellation (a new Subscription method, I presume) and the
Subscriber needs a way to cleanly cancel the subscription (a new
Subscription method, I presume) along with associated documentation.

> The Subscription::Pointer can be set to NULL for cancellation/erasure,
> set to point at another subscription, and tested for NULL.

The interesting case here is Subscribers canceling subscriptions. The
pointer to Subscription stored in Publisher is not accessible to
Subscribers. An API extension (new Subscription methods) is needed.

> The original Call pointed to is never scheduled and can be deleted
> without canceling right?.

Yes, but the API does not have a notion of the "original Call". You are
thinking of a specific API implementation called CallSubscription. I am
talking about the API itself.

> Any Calls spawned are outside the scope of the subscription itself once
> they leave the callback() method.

True, although user may need a reminder about that caveat.

>>> + virtual AsyncCall::Pointer callback() = 0;

>> Please document whether callback() can return a nil pointer and, if yes,
>> what would that mean to the Publisher. I suggest that it must not return
>> nil.
>
> It could be. Depends on the inheriting childs implementation.

My point is that it either it should be prohibited (for any
implementations) or allowed. I suggest that it is prohibited until we
support cancellations. If it is allowed now, you need to document what
Publisher is supposed to do when callback() returns nil.

Cheers,

Alex.
Received on Thu Oct 14 2010 - 23:12:53 MDT