Proposal: Registering to events if handler is not already registered #1430
Replies: 21 comments
-
This wouldn't be possible for the C# compiler (or the CLR for that matter). The event only exposes two methods, It's possible that the C# compiler could emit different code in the |
Beta Was this translation helpful? Give feedback.
-
In order to accomplish the syntax sugar I'm proposing the methods exposed by the event should be extended, to something like: add(boolean) or addIfNotPresent, and contains. |
Beta Was this translation helpful? Give feedback.
-
You can do this on the side of the event by having custom private HashSet<EventHandler> e = new HashSet<EventHandler>();
public event EventHandler E
{
add => e.Add(value);
remove => e.Remove(value);
}
private void RaiseE()
{
foreach (var handler in e)
handler(this, EventArgs.Empty);
} |
Beta Was this translation helpful? Give feedback.
-
Yes, I know. But what about the events that you consume but not declare? You end up either unregistering the handler first or use reflection to get the invocation list which ends up being unintuitive, cumbersome, risky and not performance-wise. |
Beta Was this translation helpful? Give feedback.
-
Changing the signature of the method used to add the handler couldn't be done from the subscriber side anyway. And I'm pretty sure that would be a massive breaking change, even if the CLR would allow such a signature to be used in the |
Beta Was this translation helpful? Give feedback.
-
Don't change the signature (if you do not want to use an optional parameter at the end), and implement a new one, as I said before as an alternative: "addIfNotPresent". This one would be used by: myObject.MyEvent? += MyEventHandler; And "contains" by if(myObject.MyEvent == MyEventHandler) or if(myObject.MyEvent != MyEventHandler). |
Beta Was this translation helpful? Give feedback.
-
This would require a new CLR and System.Reflection and then it would require all existing assemblies to be recompiled to declare these new accessor methods. Even then, I've read that failing to track subscription state yourself is a code smell and I'm pretty sure that's correct. I'd rather this "just in case unsubscribe" pattern was a bit in-your-face to discourage its use. |
Beta Was this translation helpful? Give feedback.
-
I understand, but the problem with this "pattern" is that it opens the door for issues, like race conditions and perf penalties. And now that many perf issues have been addressed (and solved) from C# v7.x and on, it may be time to address those code smells regarding events from within the kernel it-self. |
Beta Was this translation helpful? Give feedback.
-
I'm not sure which pattern you're referring to. The "just in case unsubscribe" pattern? It sure does have problems, not least of which is that it fundamentally causes memory leaks. |
Beta Was this translation helpful? Give feedback.
-
Never said it causes memory leaks. |
Beta Was this translation helpful? Give feedback.
-
No, I brought that up because memory leaks are one of the things that motivated me to get away from the |
Beta Was this translation helpful? Give feedback.
-
I agree with the code smell - we should be better managing object lifecycle and subscriptions, rather than doing a conditional subscribe or clear-and-subscribe. |
Beta Was this translation helpful? Give feedback.
-
This thread describes why I left |
Beta Was this translation helpful? Give feedback.
-
Is this proposal is being somehow considered? |
Beta Was this translation helpful? Give feedback.
-
@Ultrahead When things change, the issue will be updated. There is no need to go through all your issues asking for status updates. Thanks :) |
Beta Was this translation helpful? Give feedback.
-
Well, there is for me. I'm cleaning my plate and closing discarded issues. |
Beta Was this translation helpful? Give feedback.
-
@Ultrahead Issues have not been discarded. They simply haven't been triaged or assigned a specific release. |
Beta Was this translation helpful? Give feedback.
-
@Ultrahead Just so you know, everyone that follows the repo got some repetitive notifications when you did this. |
Beta Was this translation helpful? Give feedback.
-
Sorry about that, but there should a policy in place to either triage proposals in specific period of time or close them. It's been more than two years since I opened the issue and also with neither movement nor signal of interest from champions so imvho this should be closed ufn. Please respect my will as author of the proposal and do not reopen it unless the intention is to provide a meaningful discussion towards its implementation. Thanks |
Beta Was this translation helpful? Give feedback.
-
Reopening issue so it can go through the normal design process. |
Beta Was this translation helpful? Give feedback.
-
Sorry. We don't have such a policy. Design work is not based on time but on how important things are as judged by the ldm.
There are issues that go for decades without movement, up until the point they become important enough for the next release :-)
I don't see any reason to close this. It's an interesting space, and maybe there's stuff the language can do (i.e. around managing lifetimes) to make things better. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
When you register a handler to an event, the CLR does not check whether it is already registered and therefore, registration happens again.
A common practice to avoid this, is to unregister the hander first and then register it (which could bring race threats). Alternative ways can be implemented by getting the invocation list of the event (directly or through reflection).
The idea is to simplify things by allowing a sentence like, say:
This will let the CLR know that MyEventHandler must be register to the event only if NOT already registered.
Additionally, a special check could be added to the language, like:
The latter would spare us from writing code to check the invocation list of the event. And both features, will avoid the unregistering step.
Thoughts?
Beta Was this translation helpful? Give feedback.
All reactions