spinettaro / delphi-event-bus

Delphi Event Bus (for short DEB) is an Event Bus framework for Delphi
Apache License 2.0
466 stars 110 forks source link

Minor improvement of subscriber registration #52

Closed jaclas closed 3 years ago

jaclas commented 3 years ago

Please add the parameter ARaiseExcIfEmpty : Boolean to the RegisterSubscriber() method to pass it on to the FindSubscriberMethods() method. At the moment, in the RegisterSubscriber() method, when calling the FindSubscriberMethods() method, this parameter is hardcoded to True, it should not be so, see:

procedure TEventBus.RegisterSubscriber<T>(ASubscriber: TObject); <== please add ARaiseExcIfEmpty param
var
  LSubscriberClass: TClass;
  LSubscriberMethods: TArray<TSubscriberMethod>;
  LSubscriberMethod: TSubscriberMethod;
begin
  FMultiReadExclWriteSync.BeginWrite;

  try
    LSubscriberClass := ASubscriber.ClassType;
    LSubscriberMethods := TSubscribersFinder.FindSubscriberMethods<T>(LSubscriberClass, True); <==== HARDCODED
    for LSubscriberMethod in LSubscriberMethods do Subscribe<T>(ASubscriber, LSubscriberMethod);
  finally
    FMultiReadExclWriteSync.EndWrite;
  end;
end;

I have a problem with this because I have a base class that supports caching and I want to call RegisterSubscriberForEvents() in this base class (not to do this in many inherited classes). But because not every descendant cache class uses events, I get an EObjectHasNoSubscriberMethods exception in runtime.

And of course a change is necessary in these two methods

procedure TEventBus.RegisterSubscriberForChannels(ASubscriber: TObject); <== ARaiseExcIfEmpty param
begin
  RegisterSubscriber<ChannelAttribute>(ASubscriber);
end;

procedure TEventBus.RegisterSubscriberForEvents(ASubscriber: TObject); <== ARaiseExcIfEmpty param
begin
  RegisterSubscriber<SubscribeAttribute>(ASubscriber);
end;
wxinix commented 3 years ago

I created a pull-request to address your issue:

Added two new methods:

I added new methods instead of overloading the existing ones, for two reasons:

  1. Compatible with existing code
  2. Make the intention clear, that is, silently register the subscriber without throwing exceptions when the subscriber object does not have subscriber methods defined.

This should fit your needs. You might want to check out my fork: https://github.com/wxinix/delphi-event-bus

spinettaro commented 3 years ago

hi all, I agree with @wxinix : make the intention clear . I'm going to approve the PR

jaclas commented 3 years ago

thx