REQUEST: Set Context on registration

[dkounal]

A useful feature will be to allow the Context to be defined on registration (RegisterSubscriberForEvents and RegisterSubscriberForChannels) if set and override the attribute especially when using MDI forms that get events for specific MDI form.

[wxinix]

As I understand, this will break the internal data structure of the framework, and may break the code for existing applications.

The current design is to have Context specified at the level of individual subscriber methods, rather than the overall subscriber object. I think this is a good design allowing the most flexibility.

But maybe I didn't understand you correctly - can you be more specific on what exactly you need? An illustrative sample would be helpful.

------ Original Message ------ From: "Dimitris Kounalakis" notifications@github.com To: "spinettaro/delphi-event-bus" delphi-event-bus@noreply.github.com Cc: "Subscribed" subscribed@noreply.github.com Sent: 2/17/2021 4:25:40 AM Subject: [spinettaro/delphi-event-bus] REQUEST: Set Context on registration (#54)

A useful feature will to allow the Context to be defined on registration (RegisterSubscriberForEvents and RegisterSubscriberForChannels) if set and override the attribute especially when using MDI forms that get events for specific MDI form.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/spinettaro/delphi-event-bus/issues/54, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABISKGC4TKZNUD54SVZFB5LS7ODRJANCNFSM4XX7YXZA.

[dkounal]

Each subscriber issues a RegisterSubscriberForEvents and then the attributes are used to enter a new subscription. The problem with the attribute is that it is defined before compiling. I was thinking that we could have a RegisterSubscriberForEvents with an extra optional "context" argument that it used to override the hardcoded context in the attribute and save this in the Tdirectory list of subscriptions. The same should exist for unsubscription. In such a case you can have MDI forms and each MDI form can have its context during subscribing.

[dkounal]

Why I propose that? Imagine this scenario: A background thread is sending this message to a UI MDI form. Every MDI form is subscriber to this kind of message and probably a variable in the message can carry the individual MDI form's context. But it is really resource expense to send it to all MDI forms and synchronize for each MDI form. The context in such a case can be used to post the message only to the specific MDI form

[wxinix]

So basically, you'd like the "Context" to be associated with INSTANCES,
NOT TYPES?

------ Original Message ------ From: "Dimitris Kounalakis" notifications@github.com To: "spinettaro/delphi-event-bus" delphi-event-bus@noreply.github.com Cc: "Wuping Xin" xinwuping007@gmail.com; "Comment" comment@noreply.github.com Sent: 2/17/2021 9:04:07 AM Subject: Re: [spinettaro/delphi-event-bus] REQUEST: Set Context on registration (#54)

Why I propose that? Imagine this scenario: A background thread is sending this message to a UI MDI form. Every MDI form is subscriber to this kind of message and probably a variable in the message can carry the individual MDI form's context. But it is really resource expense to send it to all MDI forms and synchronize for each MDI form. The context in such a case can be used to post the message only to the specific MDI form

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spinettaro/delphi-event-bus/issues/54#issuecomment-780577823, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABISKGEXJHZDHECDTRWNYTTS7PEFPANCNFSM4XX7YXZA.

[dkounal]

I can not think of a different solution for the above scenario

[dkounal]

From what I read in the code a pair of messageClassname+context as defined in the attribute is included in the subscribers' list when subscribing. When posting also messageclassname is extracted from post arguments and context is one of the arguments. I can understand that this is a logical difference in the model but it can solve such a scenario

[dkounal]

The proposal is to allow the subscribe method to optionally set the context if in the attributes the context is empty (by instance). An other situation possible is to override the context when subscribing (to have mixed by instance and by attribute) subscribers. In both cases, if context is set by subscribing, it must be unset by unsubscribing having the same argument in Unsubscribe. That needs consistency when programming....

[wxinix]

I see. So basically, you'd like a "filtering" mechanism (using context as the filter) for different subscriber INSTANCES that are of the SAME TYPE.

------ Original Message ------ From: "Dimitris Kounalakis" notifications@github.com To: "spinettaro/delphi-event-bus" delphi-event-bus@noreply.github.com Cc: "Wuping Xin" xinwuping007@gmail.com; "Comment" comment@noreply.github.com Sent: 2/17/2021 9:32:54 AM Subject: Re: [spinettaro/delphi-event-bus] REQUEST: Set Context on registration (#54)

The proposal is to allow the subscribe method to optionally set the context if in the attributes the context is empty (by instance). An other situation possible is to override the context when subscribing (to have mixed by instance and by attribute) subscribers. In both cases, if context is set by subscribing, it must be unset by unsubscribing having the same argument in Unsubscribe. That needs consistency when programming....

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spinettaro/delphi-event-bus/issues/54#issuecomment-780595830, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABISKGGQE6ZBF7WS2PBGIILS7PHRNANCNFSM4XX7YXZA.

[dkounal]

Correct, I think it is missing part of this excellent project

[dkounal]

It could be even a separate "context" and a third part in the "classname:context" key (like "Classname:context:instancecontext") in the tdirectory subscribers' list. But this add more complexity to Post procedures

[wxinix]

I am not sure that is necessary. You can accomplish the “filtering” by using a flag field inside you event interface object, which is supported by the existing DEB framework

From a design perspective, if DEB allows the framework-level “filtering”, like you noted, basically that is mixing the user-level business logic with DEB. The user level business logic should be at user-level which you need to implement by specifying a proper interfaced Event object, and a corresponding event handler inside subscriber class.

I don’t see the need for a framework level refactoring to implement this type of instance-specific filtering. The goal of DEB is to be a light-weight and flexible framework, as my understanding.

But of course, I could be wrong or missing your points

[dkounal]

I am not sure that is necessary. You can accomplish the “filtering” by using a flag field inside you event interface object, which is supported by the existing DEB framework

Correct, I mention that, BUT: Imagine this scenario: A background thread is sending this message for an MDI form. Every MDI form instance in the program is subscriber with an interface-message that has a flag-variable inside that personalizes each actual MDI form instance. The EventBus sends to all MDI forms the same message-interface, and it does a synchronize for each MDI form instance and each form instance does the filtering. It works but it is so resource consuming.... Do you propose something alternative?

[wxinix]

It works but it is so resource consuming.... Do you propose something alternative?

I see. Your use case is clear. And I bet you have a LOT of MDI forms??? I guess you've observed significant UI delay in your case?

The efforts to refactor (or optimize) the existing DEB code base to meet your goal, probably would not be an easy and quick one, and extra test cases need to be created to make sure no existing code is broken.

[dkounal]

After the above discussion, I followed this idea:

1. I define a "MaskedContent" constant.
2. if the attribute context is this constant then the procedure TEventBus.RegisterSubscriber(ASubscriber: TObject; ARaiseExcIfEmpty: Boolean); can replace it with a "provided instance defined Context"
3. it works
4. I am looking now how to unregister it correctly

[dkounal]

I can not see any special need for unsubscribing. I issued a pull request: https://github.com/spinettaro/delphi-event-bus/pull/55

[wxinix]

I can not see any special need for unsubscribing. I issued a pull request: #55

1. The code style is not consistent with existing code.
2. If the same subscriber object is registered twice, one with the old RegisterSubscriberForEvents, one with the new RegisterCustomContextSubscriberForEvents, then Unregister would cause problem - it will remove the both registrations.

[dkounal]

2. If the same subscriber object is registered twice, one with the old RegisterSubscriberForEvents, one with the new RegisterCustomContextSubscriberForEvents, then Unregister would cause problem - it will remove the both registrations.

This is not a bug. It is a feature

[wxinix]

Generally the register and unregistered methods should be paired. Otherwise it may cause developer confusion

[spinettaro]

Hi guys,

I agree with @dkounal that this feature is quite interesting, the fact you can inject the context at runtime. We all agreed on that, there are still few things I would like to inspect in deep like the unregistration. @dkounal I'm going with @wxinix PR ( #57 ) because it maintains the code style consistent with existing code. We can review that one together :)

Thanks for the effort