convert T/i/Event<T>.Remove[All] into function:integer

Issue #151 wontfix
Arioch The created an issue

Can Events' Remove/RemoveAll subroutines be turned into functions returning how many handlers they did removed ?

That can be helpful for debugging/assertions/unittesting

For example I recently caught the following bug (stupid and trivial one - AFTER you find it).

type
  TaskParamsChangingNotification = reference to procedure( some-vars-list );

  TTaskParams = class
  public 
      ChangeListeners: iEvent< TaskParamsChangingNotification  >;
  ....

In another unit

type

  TSomeBaseClass = class
  protected
       TaskParamsChange: procedure( that-very-vars-list ); virtual; // to be redefined in derived classes
  ....

  constructor TSomeBaseClass.Create;
  begin
     ..... 
        GlobalTaskParams.ChangeListeners.Add( Self.TaskParamsChange );
        ////  ooops! What could possibly go wrong?
     .....
  end;

  destructor TSomeBaseClass.Destroy;
  begin
     ..... 
        GlobalTaskParams.ChangeListeners.RemoveAll( Self );  // removes ZERO handlers - if I only could check it
///  same unexpected result if     GlobalTaskParams.ChangeListeners.Remove( Self.TaskParamsChange );
     .....
  end;

When some object, derived from TSomeBaseClass gets destroyed it unexpectedly does NOT remove itself from the multi-event. Consequently, next attempt to change task parameters and propagate the change by IEvent<T>.Invoke causes AV.

However, if .Remove and .RemoveAll returned counter of deleted handlers that at least could be detected by unit tests or assertions.

Maybe also IEvent<T>.Add could also be a function returning some persistent token that could be used to remove the handler when needed, thus allowing anonymous methods be added to the multi-event

Comments (2)

  1. Stefan Glienke repo owner

    Although not intuitive this is a (not commonly I admit) known fact because of the way the compiler treats assignment of methods to anonymous method types - see issue #146

    If you want something more like Rx/IObservable (Subscribe returning something that can be used later to unsubscribe) then IEvent is not what you are looking for I am afraid.

  2. Arioch The reporter

    so instead of principle of least surprise we have a gotcha here

    in such a situation attempt to remove should raise an exception if nothing to remove found

  3. Log in to comment