Making an Action event also usable as Action and Action in C#Collection of Actionsfollow-up review of a custom Undo ManagerAbstract Factory ExperimentUsing a List to manage Swing event listenersCustom event handling systemSimple Event Dispatcher for GameAction<Object> callbacks using the mediator methodA C#-style Event object in C++Testing validity of Proxy servers concurrently
What is an "07" chord?
We know someone is scrying on us. Is there anything we can do about it?
What could a technologically advanced but outnumbered alien race do to destroy humanity?
What color should I use for the walls and ceiling of a photography studio?
What happens to a Bladesinger reincarnated as a Human?
Intuition behind the paradox of instantaneous heat propagation
Hypothesis testing- with normal approximation
In Cura, can I make my top and bottom layer be all perimiters?
What is the difference between an adjective and a noun modifier?
Can a planet's magnetic field be generated by non-ferromagnetic metals?
For a command to increase something, should instructions refer to the "+" key or the "=" key?
Simple code that checks if you're old enough to drive
Falcon Heavy Stage 1 Recovery
Are there any spells that aren't on any class's spell list?
why we need self-synchronization?
Adjusting the definition of a well-powered category to category theory with universes: size issues
Is a new blessing required when taking off and putting back on your tallit?
Hell0 W0rld! scored by ASCII values
My bike's adjustable stem keeps falling down
Why does Expected<T> in LLVM implement two constructors for Expected<T>&&?
Why buy a first class ticket on Southern trains?
Where do overtones in a 555 generated square wave come from?
Why is Ancient Greek "δέ" translated by Gothic "þan" /then/?
Sci-fi book trilogy about space travel & 'jacking'
Making an Action event also usable as Action and Action in C#
Collection of Actionsfollow-up review of a custom Undo ManagerAbstract Factory ExperimentUsing a List to manage Swing event listenersCustom event handling systemSimple Event Dispatcher for GameAction<Object> callbacks using the mediator methodA C#-style Event object in C++Testing validity of Proxy servers concurrently
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty
margin-bottom:0;
$begingroup$
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
Context
Before presenting the code for review, I want to be clear that I am not looking for recommendations on how to avoid having to solve the issue that the code is trying to solve in the first place
This is not production code, this is purely for exploratory purposes, and I would prefer answers that remain within the provided requirements.
With that said, let's suppose I have this contrived interface:
public interface IFoo<T>
event Action<T> ChangeValue;
event Action<object> ChangeObject;
event Action ChangeEmpty;
void InvokeChange(T value);
With the following contrived requirements:
Calling
InvokeChangewith a value of typeTthatFoowas constructed with will invoke the three eventsChangeValueis invoked with the given value as typeTChangeObjectis invoked with the given value as typeobjectChangeEmptyis invoked with no arguments
Handlers that listen to any of the three events are invoked in the global order they were added, regardless of what event they were added to.
- Effectively there is only a single event here - some 'change' happened - but 3 event signatures that can be utilized by listeners as needed.
The implementation can take any form, but must implement
IFoo<T>as it is specified above.The implementation does not need to be thread safe and can allocate memory, but no memory should leak when it is used correctly.
Example 1
The order that event listeners are called should correspond to the order in which they were added, regardless of which event they were added to:
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += () => Console.WriteLine("0) Empty!");
test.ChangeObject += (o) => Console.WriteLine("1) Object: 0", o);
test.ChangeValue += (v) => Console.WriteLine("2) Value: 0", v);
test.ChangeObject += (o) => Console.WriteLine("3) Object: 0", o);
test.ChangeEmpty += () => Console.WriteLine("4) Empty!");
test.InvokeChange(123);
Should print the following to the Console:
0) Empty!
1) Object: 123
2) Value: 123
3) Object: 123
4) Empty!
Example 2
Adding/Removing event Listeners should work as expected, while still maintaining the same order:
public static void Main()
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += EmptyHandler;
test.ChangeObject += ObjectHandler;
Console.WriteLine("1) EMPTY, OBJECT");
test.InvokeChange(1);
test.ChangeEmpty -= EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("2) OBJECT, VALUE");
test.InvokeChange(2);
test.ChangeObject -= ObjectHandler;
Console.WriteLine("3) VALUE ");
test.InvokeChange(3);
test.ChangeObject += ObjectHandler;
test.ChangeEmpty += EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("4) VALUE, OBJECT, EMPTY, VALUE");
test.InvokeChange(4);
test.ChangeValue -= ValueHandler;
test.ChangeValue -= ValueHandler;
test.ChangeEmpty -= EmptyHandler;
test.ChangeObject -= ObjectHandler;
Console.WriteLine("5) <NONE>");
test.InvokeChange(5);
static void EmptyHandler() Console.WriteLine(" - Empty!");
static void ObjectHandler(object val) Console.WriteLine(" - Object: 0", val);
static void ValueHandler(int val) Console.WriteLine(" - Value: 0", val);
Should print the following to the Console:
1) EMPTY, OBJECT
- Empty!
- Object: 1
2) OBJECT, VALUE
- Object: 2
- Value: 2
3) VALUE
- Value: 3
4) VALUE, OBJECT, EMPTY, VALUE
- Value: 4
- Object: 4
- Empty!
- Value: 4
5) <NONE>
Effectively it should perform as if there is a single invokation list that backs all 3 events.
This code is not required to be particularly performant or thread safe, but 'correct'. In other words, invocation order and frequency is correct regardless of how many times a given handler is added/removed from one of the three events.
Code For Review
With all that in mind, the solution I came up with is:
ChangeValueis a standard event backed by anAction<T>ChangeObjectandChangeEmptyare custom event accessors that provide a customaddandremoveimplementation which are ultimately also backed byChangeValue.- The
valueofChangeObject.addandChangeEmpty.addis wrapped in a lambda 'proxy' of signatureAction<T> - To ensure that the event accessors are able to also remove the corresponding proxy when in
ChangeObject.removeandChangeEmpty.remove, a dictionary is used for each to track a stack of proxies that have been bound for each value added.addpushes a proxy to the stack and is then added withChangeValue += proxyremovepops a proxy from the stack and is then removed withChangeValue -= proxy
- The
Here is the relevant code for one of the events:
public event Action<T> ChangeValue = delegate ;
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
public event Action<object> ChangeObject
add
List<Action<T>> eventProxies;
if(!ObjectEventProxies.TryGetValue(value, out eventProxies))
eventProxies = new List<Action<T>>();
ObjectEventProxies.Add(value, eventProxies);
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
remove
List<Action<T>> eventProxies;
if (ObjectEventProxies.TryGetValue(value, out eventProxies))
Action<T> proxy = eventProxies[eventProxies.Count - 1];
eventProxies.RemoveAt(eventProxies.Count - 1);
ChangeValue -= proxy;
And the full code, including the above examples: https://dotnetfiddle.net/u7NGMJ
Question
This code fulfills the given requirements but my question is, can this be simplified? There's a fair bit of boilerplate here for something that feels like it should be simpler to do.
To re-iterate, I am looking specifically for answers that still meet all of the listed requirements, regardless of how contrived or unnecessary they might seem. I should be able to swap out my IFoo<T> implementation for yours and have the output be identical.
I am aware I could just use a single Action<object> if I wanted a single signature to work for all cases. That is not what this question is asking.
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
c# event-handling
$endgroup$
add a comment
|
$begingroup$
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
Context
Before presenting the code for review, I want to be clear that I am not looking for recommendations on how to avoid having to solve the issue that the code is trying to solve in the first place
This is not production code, this is purely for exploratory purposes, and I would prefer answers that remain within the provided requirements.
With that said, let's suppose I have this contrived interface:
public interface IFoo<T>
event Action<T> ChangeValue;
event Action<object> ChangeObject;
event Action ChangeEmpty;
void InvokeChange(T value);
With the following contrived requirements:
Calling
InvokeChangewith a value of typeTthatFoowas constructed with will invoke the three eventsChangeValueis invoked with the given value as typeTChangeObjectis invoked with the given value as typeobjectChangeEmptyis invoked with no arguments
Handlers that listen to any of the three events are invoked in the global order they were added, regardless of what event they were added to.
- Effectively there is only a single event here - some 'change' happened - but 3 event signatures that can be utilized by listeners as needed.
The implementation can take any form, but must implement
IFoo<T>as it is specified above.The implementation does not need to be thread safe and can allocate memory, but no memory should leak when it is used correctly.
Example 1
The order that event listeners are called should correspond to the order in which they were added, regardless of which event they were added to:
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += () => Console.WriteLine("0) Empty!");
test.ChangeObject += (o) => Console.WriteLine("1) Object: 0", o);
test.ChangeValue += (v) => Console.WriteLine("2) Value: 0", v);
test.ChangeObject += (o) => Console.WriteLine("3) Object: 0", o);
test.ChangeEmpty += () => Console.WriteLine("4) Empty!");
test.InvokeChange(123);
Should print the following to the Console:
0) Empty!
1) Object: 123
2) Value: 123
3) Object: 123
4) Empty!
Example 2
Adding/Removing event Listeners should work as expected, while still maintaining the same order:
public static void Main()
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += EmptyHandler;
test.ChangeObject += ObjectHandler;
Console.WriteLine("1) EMPTY, OBJECT");
test.InvokeChange(1);
test.ChangeEmpty -= EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("2) OBJECT, VALUE");
test.InvokeChange(2);
test.ChangeObject -= ObjectHandler;
Console.WriteLine("3) VALUE ");
test.InvokeChange(3);
test.ChangeObject += ObjectHandler;
test.ChangeEmpty += EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("4) VALUE, OBJECT, EMPTY, VALUE");
test.InvokeChange(4);
test.ChangeValue -= ValueHandler;
test.ChangeValue -= ValueHandler;
test.ChangeEmpty -= EmptyHandler;
test.ChangeObject -= ObjectHandler;
Console.WriteLine("5) <NONE>");
test.InvokeChange(5);
static void EmptyHandler() Console.WriteLine(" - Empty!");
static void ObjectHandler(object val) Console.WriteLine(" - Object: 0", val);
static void ValueHandler(int val) Console.WriteLine(" - Value: 0", val);
Should print the following to the Console:
1) EMPTY, OBJECT
- Empty!
- Object: 1
2) OBJECT, VALUE
- Object: 2
- Value: 2
3) VALUE
- Value: 3
4) VALUE, OBJECT, EMPTY, VALUE
- Value: 4
- Object: 4
- Empty!
- Value: 4
5) <NONE>
Effectively it should perform as if there is a single invokation list that backs all 3 events.
This code is not required to be particularly performant or thread safe, but 'correct'. In other words, invocation order and frequency is correct regardless of how many times a given handler is added/removed from one of the three events.
Code For Review
With all that in mind, the solution I came up with is:
ChangeValueis a standard event backed by anAction<T>ChangeObjectandChangeEmptyare custom event accessors that provide a customaddandremoveimplementation which are ultimately also backed byChangeValue.- The
valueofChangeObject.addandChangeEmpty.addis wrapped in a lambda 'proxy' of signatureAction<T> - To ensure that the event accessors are able to also remove the corresponding proxy when in
ChangeObject.removeandChangeEmpty.remove, a dictionary is used for each to track a stack of proxies that have been bound for each value added.addpushes a proxy to the stack and is then added withChangeValue += proxyremovepops a proxy from the stack and is then removed withChangeValue -= proxy
- The
Here is the relevant code for one of the events:
public event Action<T> ChangeValue = delegate ;
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
public event Action<object> ChangeObject
add
List<Action<T>> eventProxies;
if(!ObjectEventProxies.TryGetValue(value, out eventProxies))
eventProxies = new List<Action<T>>();
ObjectEventProxies.Add(value, eventProxies);
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
remove
List<Action<T>> eventProxies;
if (ObjectEventProxies.TryGetValue(value, out eventProxies))
Action<T> proxy = eventProxies[eventProxies.Count - 1];
eventProxies.RemoveAt(eventProxies.Count - 1);
ChangeValue -= proxy;
And the full code, including the above examples: https://dotnetfiddle.net/u7NGMJ
Question
This code fulfills the given requirements but my question is, can this be simplified? There's a fair bit of boilerplate here for something that feels like it should be simpler to do.
To re-iterate, I am looking specifically for answers that still meet all of the listed requirements, regardless of how contrived or unnecessary they might seem. I should be able to swap out my IFoo<T> implementation for yours and have the output be identical.
I am aware I could just use a single Action<object> if I wanted a single signature to work for all cases. That is not what this question is asking.
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
c# event-handling
$endgroup$
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38
add a comment
|
$begingroup$
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
Context
Before presenting the code for review, I want to be clear that I am not looking for recommendations on how to avoid having to solve the issue that the code is trying to solve in the first place
This is not production code, this is purely for exploratory purposes, and I would prefer answers that remain within the provided requirements.
With that said, let's suppose I have this contrived interface:
public interface IFoo<T>
event Action<T> ChangeValue;
event Action<object> ChangeObject;
event Action ChangeEmpty;
void InvokeChange(T value);
With the following contrived requirements:
Calling
InvokeChangewith a value of typeTthatFoowas constructed with will invoke the three eventsChangeValueis invoked with the given value as typeTChangeObjectis invoked with the given value as typeobjectChangeEmptyis invoked with no arguments
Handlers that listen to any of the three events are invoked in the global order they were added, regardless of what event they were added to.
- Effectively there is only a single event here - some 'change' happened - but 3 event signatures that can be utilized by listeners as needed.
The implementation can take any form, but must implement
IFoo<T>as it is specified above.The implementation does not need to be thread safe and can allocate memory, but no memory should leak when it is used correctly.
Example 1
The order that event listeners are called should correspond to the order in which they were added, regardless of which event they were added to:
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += () => Console.WriteLine("0) Empty!");
test.ChangeObject += (o) => Console.WriteLine("1) Object: 0", o);
test.ChangeValue += (v) => Console.WriteLine("2) Value: 0", v);
test.ChangeObject += (o) => Console.WriteLine("3) Object: 0", o);
test.ChangeEmpty += () => Console.WriteLine("4) Empty!");
test.InvokeChange(123);
Should print the following to the Console:
0) Empty!
1) Object: 123
2) Value: 123
3) Object: 123
4) Empty!
Example 2
Adding/Removing event Listeners should work as expected, while still maintaining the same order:
public static void Main()
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += EmptyHandler;
test.ChangeObject += ObjectHandler;
Console.WriteLine("1) EMPTY, OBJECT");
test.InvokeChange(1);
test.ChangeEmpty -= EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("2) OBJECT, VALUE");
test.InvokeChange(2);
test.ChangeObject -= ObjectHandler;
Console.WriteLine("3) VALUE ");
test.InvokeChange(3);
test.ChangeObject += ObjectHandler;
test.ChangeEmpty += EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("4) VALUE, OBJECT, EMPTY, VALUE");
test.InvokeChange(4);
test.ChangeValue -= ValueHandler;
test.ChangeValue -= ValueHandler;
test.ChangeEmpty -= EmptyHandler;
test.ChangeObject -= ObjectHandler;
Console.WriteLine("5) <NONE>");
test.InvokeChange(5);
static void EmptyHandler() Console.WriteLine(" - Empty!");
static void ObjectHandler(object val) Console.WriteLine(" - Object: 0", val);
static void ValueHandler(int val) Console.WriteLine(" - Value: 0", val);
Should print the following to the Console:
1) EMPTY, OBJECT
- Empty!
- Object: 1
2) OBJECT, VALUE
- Object: 2
- Value: 2
3) VALUE
- Value: 3
4) VALUE, OBJECT, EMPTY, VALUE
- Value: 4
- Object: 4
- Empty!
- Value: 4
5) <NONE>
Effectively it should perform as if there is a single invokation list that backs all 3 events.
This code is not required to be particularly performant or thread safe, but 'correct'. In other words, invocation order and frequency is correct regardless of how many times a given handler is added/removed from one of the three events.
Code For Review
With all that in mind, the solution I came up with is:
ChangeValueis a standard event backed by anAction<T>ChangeObjectandChangeEmptyare custom event accessors that provide a customaddandremoveimplementation which are ultimately also backed byChangeValue.- The
valueofChangeObject.addandChangeEmpty.addis wrapped in a lambda 'proxy' of signatureAction<T> - To ensure that the event accessors are able to also remove the corresponding proxy when in
ChangeObject.removeandChangeEmpty.remove, a dictionary is used for each to track a stack of proxies that have been bound for each value added.addpushes a proxy to the stack and is then added withChangeValue += proxyremovepops a proxy from the stack and is then removed withChangeValue -= proxy
- The
Here is the relevant code for one of the events:
public event Action<T> ChangeValue = delegate ;
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
public event Action<object> ChangeObject
add
List<Action<T>> eventProxies;
if(!ObjectEventProxies.TryGetValue(value, out eventProxies))
eventProxies = new List<Action<T>>();
ObjectEventProxies.Add(value, eventProxies);
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
remove
List<Action<T>> eventProxies;
if (ObjectEventProxies.TryGetValue(value, out eventProxies))
Action<T> proxy = eventProxies[eventProxies.Count - 1];
eventProxies.RemoveAt(eventProxies.Count - 1);
ChangeValue -= proxy;
And the full code, including the above examples: https://dotnetfiddle.net/u7NGMJ
Question
This code fulfills the given requirements but my question is, can this be simplified? There's a fair bit of boilerplate here for something that feels like it should be simpler to do.
To re-iterate, I am looking specifically for answers that still meet all of the listed requirements, regardless of how contrived or unnecessary they might seem. I should be able to swap out my IFoo<T> implementation for yours and have the output be identical.
I am aware I could just use a single Action<object> if I wanted a single signature to work for all cases. That is not what this question is asking.
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
c# event-handling
$endgroup$
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
Context
Before presenting the code for review, I want to be clear that I am not looking for recommendations on how to avoid having to solve the issue that the code is trying to solve in the first place
This is not production code, this is purely for exploratory purposes, and I would prefer answers that remain within the provided requirements.
With that said, let's suppose I have this contrived interface:
public interface IFoo<T>
event Action<T> ChangeValue;
event Action<object> ChangeObject;
event Action ChangeEmpty;
void InvokeChange(T value);
With the following contrived requirements:
Calling
InvokeChangewith a value of typeTthatFoowas constructed with will invoke the three eventsChangeValueis invoked with the given value as typeTChangeObjectis invoked with the given value as typeobjectChangeEmptyis invoked with no arguments
Handlers that listen to any of the three events are invoked in the global order they were added, regardless of what event they were added to.
- Effectively there is only a single event here - some 'change' happened - but 3 event signatures that can be utilized by listeners as needed.
The implementation can take any form, but must implement
IFoo<T>as it is specified above.The implementation does not need to be thread safe and can allocate memory, but no memory should leak when it is used correctly.
Example 1
The order that event listeners are called should correspond to the order in which they were added, regardless of which event they were added to:
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += () => Console.WriteLine("0) Empty!");
test.ChangeObject += (o) => Console.WriteLine("1) Object: 0", o);
test.ChangeValue += (v) => Console.WriteLine("2) Value: 0", v);
test.ChangeObject += (o) => Console.WriteLine("3) Object: 0", o);
test.ChangeEmpty += () => Console.WriteLine("4) Empty!");
test.InvokeChange(123);
Should print the following to the Console:
0) Empty!
1) Object: 123
2) Value: 123
3) Object: 123
4) Empty!
Example 2
Adding/Removing event Listeners should work as expected, while still maintaining the same order:
public static void Main()
IFoo<int> test = new Foo<int>();
test.ChangeEmpty += EmptyHandler;
test.ChangeObject += ObjectHandler;
Console.WriteLine("1) EMPTY, OBJECT");
test.InvokeChange(1);
test.ChangeEmpty -= EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("2) OBJECT, VALUE");
test.InvokeChange(2);
test.ChangeObject -= ObjectHandler;
Console.WriteLine("3) VALUE ");
test.InvokeChange(3);
test.ChangeObject += ObjectHandler;
test.ChangeEmpty += EmptyHandler;
test.ChangeValue += ValueHandler;
Console.WriteLine("4) VALUE, OBJECT, EMPTY, VALUE");
test.InvokeChange(4);
test.ChangeValue -= ValueHandler;
test.ChangeValue -= ValueHandler;
test.ChangeEmpty -= EmptyHandler;
test.ChangeObject -= ObjectHandler;
Console.WriteLine("5) <NONE>");
test.InvokeChange(5);
static void EmptyHandler() Console.WriteLine(" - Empty!");
static void ObjectHandler(object val) Console.WriteLine(" - Object: 0", val);
static void ValueHandler(int val) Console.WriteLine(" - Value: 0", val);
Should print the following to the Console:
1) EMPTY, OBJECT
- Empty!
- Object: 1
2) OBJECT, VALUE
- Object: 2
- Value: 2
3) VALUE
- Value: 3
4) VALUE, OBJECT, EMPTY, VALUE
- Value: 4
- Object: 4
- Empty!
- Value: 4
5) <NONE>
Effectively it should perform as if there is a single invokation list that backs all 3 events.
This code is not required to be particularly performant or thread safe, but 'correct'. In other words, invocation order and frequency is correct regardless of how many times a given handler is added/removed from one of the three events.
Code For Review
With all that in mind, the solution I came up with is:
ChangeValueis a standard event backed by anAction<T>ChangeObjectandChangeEmptyare custom event accessors that provide a customaddandremoveimplementation which are ultimately also backed byChangeValue.- The
valueofChangeObject.addandChangeEmpty.addis wrapped in a lambda 'proxy' of signatureAction<T> - To ensure that the event accessors are able to also remove the corresponding proxy when in
ChangeObject.removeandChangeEmpty.remove, a dictionary is used for each to track a stack of proxies that have been bound for each value added.addpushes a proxy to the stack and is then added withChangeValue += proxyremovepops a proxy from the stack and is then removed withChangeValue -= proxy
- The
Here is the relevant code for one of the events:
public event Action<T> ChangeValue = delegate ;
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
public event Action<object> ChangeObject
add
List<Action<T>> eventProxies;
if(!ObjectEventProxies.TryGetValue(value, out eventProxies))
eventProxies = new List<Action<T>>();
ObjectEventProxies.Add(value, eventProxies);
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
remove
List<Action<T>> eventProxies;
if (ObjectEventProxies.TryGetValue(value, out eventProxies))
Action<T> proxy = eventProxies[eventProxies.Count - 1];
eventProxies.RemoveAt(eventProxies.Count - 1);
ChangeValue -= proxy;
And the full code, including the above examples: https://dotnetfiddle.net/u7NGMJ
Question
This code fulfills the given requirements but my question is, can this be simplified? There's a fair bit of boilerplate here for something that feels like it should be simpler to do.
To re-iterate, I am looking specifically for answers that still meet all of the listed requirements, regardless of how contrived or unnecessary they might seem. I should be able to swap out my IFoo<T> implementation for yours and have the output be identical.
I am aware I could just use a single Action<object> if I wanted a single signature to work for all cases. That is not what this question is asking.
Edit
I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659
c# event-handling
c# event-handling
edited Jul 15 at 22:14
Johannes
asked Jun 1 at 4:28
JohannesJohannes
1976 bronze badges
1976 bronze badges
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38
add a comment
|
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38
add a comment
|
4 Answers
4
active
oldest
votes
$begingroup$
IMO your events should accommodate the C# standard for events:
public interface IFoo<T>
event EventHandler<T> ChangeValue;
event EventHandler<object> ChangeObject;
event EventHandler ChangeEmpty;
void InvokeChange(T value);
The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.
You can modify dfhwze's solution slightly to meet all your requirements to:
public class Foo<T> : IFoo<T>
private List<object> handlers = new List<object>();
void AddHandler(object handler)
handlers.Add(handler);
void RemoveHandler(object handler)
int index = handlers.LastIndexOf(handler);
if (index >= 0)
handlers.RemoveAt(index);
public event EventHandler<T> ChangeValue add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value); remove => RemoveHandler(value);
public void InvokeChange(T value)
foreach (object handler in handlers)
Invoke((dynamic)handler, value);
public void Invoke(EventHandler handler, T value)
handler?.Invoke(this, EventArgs.Empty);
public void Invoke<S>(EventHandler<S> handler, S value)
handler?.Invoke(this, value);
A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.
Another solution building on your own could be:
public class Foo<T> : IFoo<T>
private event EventHandler<T> AllEvents;
private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();
void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
if (!allHandlers.TryGetValue(source, out var handler))
handler = (handlerFactory(), 1);
else
handler.Count++;
allHandlers[source] = handler;
AllEvents += handler.Handler;
void RemoveHandler(Delegate source)
if (allHandlers.TryGetValue(source, out var handler))
handler.Count--;
AllEvents -= handler.Handler;
if (handler.Count <= 0)
allHandlers.Remove(source);
else
allHandlers[source] = handler;
public event EventHandler<T> ChangeValue add => AddHandler(value, () => value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value);
public void InvokeChange(T value)
AllEvents?.Invoke(this, value);
public void Clear()
allHandlers.Clear();
AllEvents = null;
$endgroup$
$begingroup$
Your point regarding the C# event standard is valid, but this does modify theIFoo<T>interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy
$endgroup$
– Johannes
Jun 2 at 6:00
add a comment
|
$begingroup$
no memory should leak when it is used correctly.
Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).
dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:
public void InvokeChange(T value)
foreach (var listener in listeners)
if (listener is Action a)
a();
else if (listener is Action<T> aT)
aT(value);
else if (listener is Action<object> aObject)
aObject((object)value);
else
throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.
Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.
Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:
add
Action<T> proxy = /* whatever */;
WhateverEventProxies.Add(value, proxy);
ChangeValue += proxy;
remove
var proxity = WhateverEventProxies.Remove(value);
ChangeValue -= proxy;
All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.
One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).
Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.
$endgroup$
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:Action<T>,Action<object>andAction.
$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
add a comment
|
$begingroup$
Review
You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.
add
// ..
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
Proposed Solution
There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.
Store all listeners in a LinkedList (allow for duplicates).
private LinkedList<object> listeners = new LinkedList<object>();
Register/Unregister event listeners without using boiler-plate code.
private void Register(object listener)
if (listener == null) return;
listeners.AddFirst(listener);
private void Unregister(object listener)
if (listener == null) return;
listeners.Remove(listener);
public event Action<T> ChangeValue
add => Register(value);
remove => Unregister(value);
public event Action<object> ChangeObject
add => Register(value);
remove => Unregister(value);
public event Action ChangeEmpty
add => Register(value);
remove => Unregister(value);
Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..
public void InvokeChange(T value)
foreach (var listener in listeners.Reverse())
Invoke((dynamic)listener, value);
// these methods require equivalent method signatures ->
// -> see comments that Action<T> and Action<object> can be combined!
private void Invoke(Action<T> action, T value)
action.Invoke(value);
private void Invoke(Action<object> action, T value)
action.Invoke(value);
private void Invoke(Action action, T value)
action.Invoke();
private void Invoke(Object unsupportedEventListenerType, T value)
// ignore or throw exception ..
$endgroup$
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
$begingroup$
You can combineInvoke(Action<T> action, T value)andInvoke(Action<object> action, T value)in one:Invoke<S>(Action<S> action, S value)
$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
|
show 3 more comments
$begingroup$
Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.
Thank you specifically for solutions by
dfhwze - using a single backing collection instead of one for each event
Henrik Hansen - using reference counting to avoid needing a proxyListfor each handler.
VisualMelon - separating proxy management logic as its own object.
I've compiled my own approach that pulls from each of these answers
A Utility for Managing the event proxies
I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.
As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.
My implementation of the Event Proxy Utility object is:
public struct EventProxyContainer<T>
private struct EventProxy
public Action<T> proxy;
public int count;
private Dictionary<object, EventProxy> handlerProxies;
public Action<T> Add(object handler) /* See Below */
public Action<T> Remove(object handler) /* See Below */
Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.
This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.
Add Implementation
public Action<T> Add(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count += 1;
handlerProxies[handler] = entry;
else
entry = new EventProxy() count = 1 ;
if(handler is Action<object>)
entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
else if (handler is Action)
entry.proxy = (v) => ((Action)handler).Invoke();
handlerProxies.Add(handler, entry);
return entry.proxy;
Remove implementation
Again we early out if the handler is Action<T>
public Action<T> Remove(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count -= 1;
if(entry.count == 0)
handlerProxies.Remove(handler);
else
handlerProxies[handler] = entry;
return entry.proxy;
Foo<T> Implementation
This really cleans up the Foo<T> implementation quite nicely:
public class Foo<T> : IFoo<T>
private EventProxyContainer<T> changeProxy;
public event Action<T> ChangeValue = delegate ;
public event Action<object> ChangeObject
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public event Action ChangeEmpty
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public void InvokeChange(T value)
ChangeValue.Invoke(value);
I like this approach because
- It still satisfies all the original requirements, and produces the same output as my original examples.
- It can be retrofitted in any case where you have an event of type
Action<T>where you also want event listeners to be able to use it asAction<object>andActioninstead. - The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in
IFoo. - Reference counting our proxies allows us to limit one memory allocation per Unique Handler
- We only construct a proxy for
Action<object>andActionhandlers -Action<T>handlers are added to the backing event object as normal. AddandRemovewhitelist onlyAction<object>andAction, and return null in all other cases, whichevent +=andevent -=handles gracefully.
The updated code with examples can be found as a DotNetFiddle here, and as a gist here.
$endgroup$
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
add a comment
|
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/4.0/"u003ecc by-sa 4.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221457%2fmaking-an-actiont-event-also-usable-as-actionobject-and-action-in-c%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
4 Answers
4
active
oldest
votes
4 Answers
4
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
IMO your events should accommodate the C# standard for events:
public interface IFoo<T>
event EventHandler<T> ChangeValue;
event EventHandler<object> ChangeObject;
event EventHandler ChangeEmpty;
void InvokeChange(T value);
The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.
You can modify dfhwze's solution slightly to meet all your requirements to:
public class Foo<T> : IFoo<T>
private List<object> handlers = new List<object>();
void AddHandler(object handler)
handlers.Add(handler);
void RemoveHandler(object handler)
int index = handlers.LastIndexOf(handler);
if (index >= 0)
handlers.RemoveAt(index);
public event EventHandler<T> ChangeValue add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value); remove => RemoveHandler(value);
public void InvokeChange(T value)
foreach (object handler in handlers)
Invoke((dynamic)handler, value);
public void Invoke(EventHandler handler, T value)
handler?.Invoke(this, EventArgs.Empty);
public void Invoke<S>(EventHandler<S> handler, S value)
handler?.Invoke(this, value);
A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.
Another solution building on your own could be:
public class Foo<T> : IFoo<T>
private event EventHandler<T> AllEvents;
private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();
void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
if (!allHandlers.TryGetValue(source, out var handler))
handler = (handlerFactory(), 1);
else
handler.Count++;
allHandlers[source] = handler;
AllEvents += handler.Handler;
void RemoveHandler(Delegate source)
if (allHandlers.TryGetValue(source, out var handler))
handler.Count--;
AllEvents -= handler.Handler;
if (handler.Count <= 0)
allHandlers.Remove(source);
else
allHandlers[source] = handler;
public event EventHandler<T> ChangeValue add => AddHandler(value, () => value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value);
public void InvokeChange(T value)
AllEvents?.Invoke(this, value);
public void Clear()
allHandlers.Clear();
AllEvents = null;
$endgroup$
$begingroup$
Your point regarding the C# event standard is valid, but this does modify theIFoo<T>interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy
$endgroup$
– Johannes
Jun 2 at 6:00
add a comment
|
$begingroup$
IMO your events should accommodate the C# standard for events:
public interface IFoo<T>
event EventHandler<T> ChangeValue;
event EventHandler<object> ChangeObject;
event EventHandler ChangeEmpty;
void InvokeChange(T value);
The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.
You can modify dfhwze's solution slightly to meet all your requirements to:
public class Foo<T> : IFoo<T>
private List<object> handlers = new List<object>();
void AddHandler(object handler)
handlers.Add(handler);
void RemoveHandler(object handler)
int index = handlers.LastIndexOf(handler);
if (index >= 0)
handlers.RemoveAt(index);
public event EventHandler<T> ChangeValue add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value); remove => RemoveHandler(value);
public void InvokeChange(T value)
foreach (object handler in handlers)
Invoke((dynamic)handler, value);
public void Invoke(EventHandler handler, T value)
handler?.Invoke(this, EventArgs.Empty);
public void Invoke<S>(EventHandler<S> handler, S value)
handler?.Invoke(this, value);
A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.
Another solution building on your own could be:
public class Foo<T> : IFoo<T>
private event EventHandler<T> AllEvents;
private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();
void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
if (!allHandlers.TryGetValue(source, out var handler))
handler = (handlerFactory(), 1);
else
handler.Count++;
allHandlers[source] = handler;
AllEvents += handler.Handler;
void RemoveHandler(Delegate source)
if (allHandlers.TryGetValue(source, out var handler))
handler.Count--;
AllEvents -= handler.Handler;
if (handler.Count <= 0)
allHandlers.Remove(source);
else
allHandlers[source] = handler;
public event EventHandler<T> ChangeValue add => AddHandler(value, () => value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value);
public void InvokeChange(T value)
AllEvents?.Invoke(this, value);
public void Clear()
allHandlers.Clear();
AllEvents = null;
$endgroup$
$begingroup$
Your point regarding the C# event standard is valid, but this does modify theIFoo<T>interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy
$endgroup$
– Johannes
Jun 2 at 6:00
add a comment
|
$begingroup$
IMO your events should accommodate the C# standard for events:
public interface IFoo<T>
event EventHandler<T> ChangeValue;
event EventHandler<object> ChangeObject;
event EventHandler ChangeEmpty;
void InvokeChange(T value);
The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.
You can modify dfhwze's solution slightly to meet all your requirements to:
public class Foo<T> : IFoo<T>
private List<object> handlers = new List<object>();
void AddHandler(object handler)
handlers.Add(handler);
void RemoveHandler(object handler)
int index = handlers.LastIndexOf(handler);
if (index >= 0)
handlers.RemoveAt(index);
public event EventHandler<T> ChangeValue add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value); remove => RemoveHandler(value);
public void InvokeChange(T value)
foreach (object handler in handlers)
Invoke((dynamic)handler, value);
public void Invoke(EventHandler handler, T value)
handler?.Invoke(this, EventArgs.Empty);
public void Invoke<S>(EventHandler<S> handler, S value)
handler?.Invoke(this, value);
A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.
Another solution building on your own could be:
public class Foo<T> : IFoo<T>
private event EventHandler<T> AllEvents;
private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();
void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
if (!allHandlers.TryGetValue(source, out var handler))
handler = (handlerFactory(), 1);
else
handler.Count++;
allHandlers[source] = handler;
AllEvents += handler.Handler;
void RemoveHandler(Delegate source)
if (allHandlers.TryGetValue(source, out var handler))
handler.Count--;
AllEvents -= handler.Handler;
if (handler.Count <= 0)
allHandlers.Remove(source);
else
allHandlers[source] = handler;
public event EventHandler<T> ChangeValue add => AddHandler(value, () => value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value);
public void InvokeChange(T value)
AllEvents?.Invoke(this, value);
public void Clear()
allHandlers.Clear();
AllEvents = null;
$endgroup$
IMO your events should accommodate the C# standard for events:
public interface IFoo<T>
event EventHandler<T> ChangeValue;
event EventHandler<object> ChangeObject;
event EventHandler ChangeEmpty;
void InvokeChange(T value);
The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.
You can modify dfhwze's solution slightly to meet all your requirements to:
public class Foo<T> : IFoo<T>
private List<object> handlers = new List<object>();
void AddHandler(object handler)
handlers.Add(handler);
void RemoveHandler(object handler)
int index = handlers.LastIndexOf(handler);
if (index >= 0)
handlers.RemoveAt(index);
public event EventHandler<T> ChangeValue add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value); remove => RemoveHandler(value);
public void InvokeChange(T value)
foreach (object handler in handlers)
Invoke((dynamic)handler, value);
public void Invoke(EventHandler handler, T value)
handler?.Invoke(this, EventArgs.Empty);
public void Invoke<S>(EventHandler<S> handler, S value)
handler?.Invoke(this, value);
A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.
Another solution building on your own could be:
public class Foo<T> : IFoo<T>
private event EventHandler<T> AllEvents;
private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();
void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
if (!allHandlers.TryGetValue(source, out var handler))
handler = (handlerFactory(), 1);
else
handler.Count++;
allHandlers[source] = handler;
AllEvents += handler.Handler;
void RemoveHandler(Delegate source)
if (allHandlers.TryGetValue(source, out var handler))
handler.Count--;
AllEvents -= handler.Handler;
if (handler.Count <= 0)
allHandlers.Remove(source);
else
allHandlers[source] = handler;
public event EventHandler<T> ChangeValue add => AddHandler(value, () => value); remove => RemoveHandler(value);
public event EventHandler<object> ChangeObject add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value);
public event EventHandler ChangeEmpty add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value);
public void InvokeChange(T value)
AllEvents?.Invoke(this, value);
public void Clear()
allHandlers.Clear();
AllEvents = null;
answered Jun 1 at 11:15
Henrik HansenHenrik Hansen
12.7k1 gold badge18 silver badges42 bronze badges
12.7k1 gold badge18 silver badges42 bronze badges
$begingroup$
Your point regarding the C# event standard is valid, but this does modify theIFoo<T>interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy
$endgroup$
– Johannes
Jun 2 at 6:00
add a comment
|
$begingroup$
Your point regarding the C# event standard is valid, but this does modify theIFoo<T>interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy
$endgroup$
– Johannes
Jun 2 at 6:00
$begingroup$
Your point regarding the C# event standard is valid, but this does modify the
IFoo<T> interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy$endgroup$
– Johannes
Jun 2 at 6:00
$begingroup$
Your point regarding the C# event standard is valid, but this does modify the
IFoo<T> interface, which for the sake of this specific question means it's insufficient. I do like the simpler approach of effectively reference counting the proxy$endgroup$
– Johannes
Jun 2 at 6:00
add a comment
|
$begingroup$
no memory should leak when it is used correctly.
Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).
dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:
public void InvokeChange(T value)
foreach (var listener in listeners)
if (listener is Action a)
a();
else if (listener is Action<T> aT)
aT(value);
else if (listener is Action<object> aObject)
aObject((object)value);
else
throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.
Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.
Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:
add
Action<T> proxy = /* whatever */;
WhateverEventProxies.Add(value, proxy);
ChangeValue += proxy;
remove
var proxity = WhateverEventProxies.Remove(value);
ChangeValue -= proxy;
All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.
One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).
Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.
$endgroup$
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:Action<T>,Action<object>andAction.
$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
add a comment
|
$begingroup$
no memory should leak when it is used correctly.
Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).
dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:
public void InvokeChange(T value)
foreach (var listener in listeners)
if (listener is Action a)
a();
else if (listener is Action<T> aT)
aT(value);
else if (listener is Action<object> aObject)
aObject((object)value);
else
throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.
Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.
Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:
add
Action<T> proxy = /* whatever */;
WhateverEventProxies.Add(value, proxy);
ChangeValue += proxy;
remove
var proxity = WhateverEventProxies.Remove(value);
ChangeValue -= proxy;
All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.
One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).
Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.
$endgroup$
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:Action<T>,Action<object>andAction.
$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
add a comment
|
$begingroup$
no memory should leak when it is used correctly.
Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).
dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:
public void InvokeChange(T value)
foreach (var listener in listeners)
if (listener is Action a)
a();
else if (listener is Action<T> aT)
aT(value);
else if (listener is Action<object> aObject)
aObject((object)value);
else
throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.
Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.
Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:
add
Action<T> proxy = /* whatever */;
WhateverEventProxies.Add(value, proxy);
ChangeValue += proxy;
remove
var proxity = WhateverEventProxies.Remove(value);
ChangeValue -= proxy;
All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.
One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).
Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.
$endgroup$
no memory should leak when it is used correctly.
Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).
dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:
public void InvokeChange(T value)
foreach (var listener in listeners)
if (listener is Action a)
a();
else if (listener is Action<T> aT)
aT(value);
else if (listener is Action<object> aObject)
aObject((object)value);
else
throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.
Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.
Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:
add
Action<T> proxy = /* whatever */;
WhateverEventProxies.Add(value, proxy);
ChangeValue += proxy;
remove
var proxity = WhateverEventProxies.Remove(value);
ChangeValue -= proxy;
All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.
One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).
Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.
answered Jun 1 at 13:34
VisualMelonVisualMelon
6,76515 silver badges44 bronze badges
6,76515 silver badges44 bronze badges
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:Action<T>,Action<object>andAction.
$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
add a comment
|
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:Action<T>,Action<object>andAction.
$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
For enterprise applications, I find the switch also a better alternative than the dynamic approach. But for the sake of this challenge, I wanted to point out this unorthodox possibility.
$endgroup$
– dfhwze
Jun 1 at 13:44
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:
Action<T>, Action<object> and Action.$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
Explicitly checking for the type is definitely the best approach for that specifically since (at least within the scope of this question) there are only 3 valid types:
Action<T>, Action<object> and Action.$endgroup$
– Johannes
Jun 2 at 6:07
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
$begingroup$
I really like the idea of moving all the boilerplate logic to a helper object. I've taken parts of each answer posted so far and added my own second iteration - I've realized that in my specific use case you can handle add type checking in Add/Remove and don't have to worry about that in Invoke. Thank you for taking the time to post your answer, it was very useful :)
$endgroup$
– Johannes
Jun 2 at 8:37
add a comment
|
$begingroup$
Review
You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.
add
// ..
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
Proposed Solution
There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.
Store all listeners in a LinkedList (allow for duplicates).
private LinkedList<object> listeners = new LinkedList<object>();
Register/Unregister event listeners without using boiler-plate code.
private void Register(object listener)
if (listener == null) return;
listeners.AddFirst(listener);
private void Unregister(object listener)
if (listener == null) return;
listeners.Remove(listener);
public event Action<T> ChangeValue
add => Register(value);
remove => Unregister(value);
public event Action<object> ChangeObject
add => Register(value);
remove => Unregister(value);
public event Action ChangeEmpty
add => Register(value);
remove => Unregister(value);
Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..
public void InvokeChange(T value)
foreach (var listener in listeners.Reverse())
Invoke((dynamic)listener, value);
// these methods require equivalent method signatures ->
// -> see comments that Action<T> and Action<object> can be combined!
private void Invoke(Action<T> action, T value)
action.Invoke(value);
private void Invoke(Action<object> action, T value)
action.Invoke(value);
private void Invoke(Action action, T value)
action.Invoke();
private void Invoke(Object unsupportedEventListenerType, T value)
// ignore or throw exception ..
$endgroup$
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
$begingroup$
You can combineInvoke(Action<T> action, T value)andInvoke(Action<object> action, T value)in one:Invoke<S>(Action<S> action, S value)
$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
|
show 3 more comments
$begingroup$
Review
You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.
add
// ..
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
Proposed Solution
There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.
Store all listeners in a LinkedList (allow for duplicates).
private LinkedList<object> listeners = new LinkedList<object>();
Register/Unregister event listeners without using boiler-plate code.
private void Register(object listener)
if (listener == null) return;
listeners.AddFirst(listener);
private void Unregister(object listener)
if (listener == null) return;
listeners.Remove(listener);
public event Action<T> ChangeValue
add => Register(value);
remove => Unregister(value);
public event Action<object> ChangeObject
add => Register(value);
remove => Unregister(value);
public event Action ChangeEmpty
add => Register(value);
remove => Unregister(value);
Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..
public void InvokeChange(T value)
foreach (var listener in listeners.Reverse())
Invoke((dynamic)listener, value);
// these methods require equivalent method signatures ->
// -> see comments that Action<T> and Action<object> can be combined!
private void Invoke(Action<T> action, T value)
action.Invoke(value);
private void Invoke(Action<object> action, T value)
action.Invoke(value);
private void Invoke(Action action, T value)
action.Invoke();
private void Invoke(Object unsupportedEventListenerType, T value)
// ignore or throw exception ..
$endgroup$
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
$begingroup$
You can combineInvoke(Action<T> action, T value)andInvoke(Action<object> action, T value)in one:Invoke<S>(Action<S> action, S value)
$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
|
show 3 more comments
$begingroup$
Review
You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.
add
// ..
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
Proposed Solution
There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.
Store all listeners in a LinkedList (allow for duplicates).
private LinkedList<object> listeners = new LinkedList<object>();
Register/Unregister event listeners without using boiler-plate code.
private void Register(object listener)
if (listener == null) return;
listeners.AddFirst(listener);
private void Unregister(object listener)
if (listener == null) return;
listeners.Remove(listener);
public event Action<T> ChangeValue
add => Register(value);
remove => Unregister(value);
public event Action<object> ChangeObject
add => Register(value);
remove => Unregister(value);
public event Action ChangeEmpty
add => Register(value);
remove => Unregister(value);
Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..
public void InvokeChange(T value)
foreach (var listener in listeners.Reverse())
Invoke((dynamic)listener, value);
// these methods require equivalent method signatures ->
// -> see comments that Action<T> and Action<object> can be combined!
private void Invoke(Action<T> action, T value)
action.Invoke(value);
private void Invoke(Action<object> action, T value)
action.Invoke(value);
private void Invoke(Action action, T value)
action.Invoke();
private void Invoke(Object unsupportedEventListenerType, T value)
// ignore or throw exception ..
$endgroup$
Review
You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.
add
// ..
Action<T> proxy = (T v) => value.Invoke(v);
eventProxies.Add(proxy);
ChangeValue += proxy;
The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.
private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
= new Dictionary<Action<object>, List<Action<T>>>();
Proposed Solution
There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.
Store all listeners in a LinkedList (allow for duplicates).
private LinkedList<object> listeners = new LinkedList<object>();
Register/Unregister event listeners without using boiler-plate code.
private void Register(object listener)
if (listener == null) return;
listeners.AddFirst(listener);
private void Unregister(object listener)
if (listener == null) return;
listeners.Remove(listener);
public event Action<T> ChangeValue
add => Register(value);
remove => Unregister(value);
public event Action<object> ChangeObject
add => Register(value);
remove => Unregister(value);
public event Action ChangeEmpty
add => Register(value);
remove => Unregister(value);
Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..
public void InvokeChange(T value)
foreach (var listener in listeners.Reverse())
Invoke((dynamic)listener, value);
// these methods require equivalent method signatures ->
// -> see comments that Action<T> and Action<object> can be combined!
private void Invoke(Action<T> action, T value)
action.Invoke(value);
private void Invoke(Action<object> action, T value)
action.Invoke(value);
private void Invoke(Action action, T value)
action.Invoke();
private void Invoke(Object unsupportedEventListenerType, T value)
// ignore or throw exception ..
edited Jun 2 at 7:41
answered Jun 1 at 9:12
dfhwzedfhwze
13k3 gold badges24 silver badges94 bronze badges
13k3 gold badges24 silver badges94 bronze badges
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
$begingroup$
You can combineInvoke(Action<T> action, T value)andInvoke(Action<object> action, T value)in one:Invoke<S>(Action<S> action, S value)
$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
|
show 3 more comments
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
$begingroup$
You can combineInvoke(Action<T> action, T value)andInvoke(Action<object> action, T value)in one:Invoke<S>(Action<S> action, S value)
$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
1
1
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
$begingroup$
This solution is not doing exactly what's required, because if you bind the same delegate instance twice or more, it'll only be called once and hence the global call order is somewhat broken. But why he wants to have a possible handler called more times in the same invocation is not clear from the question?
$endgroup$
– Henrik Hansen
Jun 1 at 10:55
1
1
$begingroup$
You can combine
Invoke(Action<T> action, T value) and Invoke(Action<object> action, T value) in one: Invoke<S>(Action<S> action, S value)$endgroup$
– Henrik Hansen
Jun 1 at 10:58
$begingroup$
You can combine
Invoke(Action<T> action, T value) and Invoke(Action<object> action, T value) in one: Invoke<S>(Action<S> action, S value)$endgroup$
– Henrik Hansen
Jun 1 at 10:58
2
2
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
$begingroup$
@HenrikHansen (1) If he wants the exact same handler registered more than once, the hash set must be replaced with an ordinary list. I overlooked this as a requirement. (2) Nice one!
$endgroup$
– dfhwze
Jun 1 at 11:14
1
1
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
$begingroup$
@Johannes Thanks for the clarification. I will no longer update my anwer because I feel better answers are available now.
$endgroup$
– dfhwze
Jun 2 at 7:07
1
1
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
$begingroup$
@Johannes That being said, I had to change it one more time just to meet the requirements.
$endgroup$
– dfhwze
Jun 2 at 7:42
|
show 3 more comments
$begingroup$
Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.
Thank you specifically for solutions by
dfhwze - using a single backing collection instead of one for each event
Henrik Hansen - using reference counting to avoid needing a proxyListfor each handler.
VisualMelon - separating proxy management logic as its own object.
I've compiled my own approach that pulls from each of these answers
A Utility for Managing the event proxies
I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.
As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.
My implementation of the Event Proxy Utility object is:
public struct EventProxyContainer<T>
private struct EventProxy
public Action<T> proxy;
public int count;
private Dictionary<object, EventProxy> handlerProxies;
public Action<T> Add(object handler) /* See Below */
public Action<T> Remove(object handler) /* See Below */
Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.
This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.
Add Implementation
public Action<T> Add(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count += 1;
handlerProxies[handler] = entry;
else
entry = new EventProxy() count = 1 ;
if(handler is Action<object>)
entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
else if (handler is Action)
entry.proxy = (v) => ((Action)handler).Invoke();
handlerProxies.Add(handler, entry);
return entry.proxy;
Remove implementation
Again we early out if the handler is Action<T>
public Action<T> Remove(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count -= 1;
if(entry.count == 0)
handlerProxies.Remove(handler);
else
handlerProxies[handler] = entry;
return entry.proxy;
Foo<T> Implementation
This really cleans up the Foo<T> implementation quite nicely:
public class Foo<T> : IFoo<T>
private EventProxyContainer<T> changeProxy;
public event Action<T> ChangeValue = delegate ;
public event Action<object> ChangeObject
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public event Action ChangeEmpty
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public void InvokeChange(T value)
ChangeValue.Invoke(value);
I like this approach because
- It still satisfies all the original requirements, and produces the same output as my original examples.
- It can be retrofitted in any case where you have an event of type
Action<T>where you also want event listeners to be able to use it asAction<object>andActioninstead. - The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in
IFoo. - Reference counting our proxies allows us to limit one memory allocation per Unique Handler
- We only construct a proxy for
Action<object>andActionhandlers -Action<T>handlers are added to the backing event object as normal. AddandRemovewhitelist onlyAction<object>andAction, and return null in all other cases, whichevent +=andevent -=handles gracefully.
The updated code with examples can be found as a DotNetFiddle here, and as a gist here.
$endgroup$
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
add a comment
|
$begingroup$
Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.
Thank you specifically for solutions by
dfhwze - using a single backing collection instead of one for each event
Henrik Hansen - using reference counting to avoid needing a proxyListfor each handler.
VisualMelon - separating proxy management logic as its own object.
I've compiled my own approach that pulls from each of these answers
A Utility for Managing the event proxies
I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.
As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.
My implementation of the Event Proxy Utility object is:
public struct EventProxyContainer<T>
private struct EventProxy
public Action<T> proxy;
public int count;
private Dictionary<object, EventProxy> handlerProxies;
public Action<T> Add(object handler) /* See Below */
public Action<T> Remove(object handler) /* See Below */
Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.
This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.
Add Implementation
public Action<T> Add(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count += 1;
handlerProxies[handler] = entry;
else
entry = new EventProxy() count = 1 ;
if(handler is Action<object>)
entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
else if (handler is Action)
entry.proxy = (v) => ((Action)handler).Invoke();
handlerProxies.Add(handler, entry);
return entry.proxy;
Remove implementation
Again we early out if the handler is Action<T>
public Action<T> Remove(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count -= 1;
if(entry.count == 0)
handlerProxies.Remove(handler);
else
handlerProxies[handler] = entry;
return entry.proxy;
Foo<T> Implementation
This really cleans up the Foo<T> implementation quite nicely:
public class Foo<T> : IFoo<T>
private EventProxyContainer<T> changeProxy;
public event Action<T> ChangeValue = delegate ;
public event Action<object> ChangeObject
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public event Action ChangeEmpty
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public void InvokeChange(T value)
ChangeValue.Invoke(value);
I like this approach because
- It still satisfies all the original requirements, and produces the same output as my original examples.
- It can be retrofitted in any case where you have an event of type
Action<T>where you also want event listeners to be able to use it asAction<object>andActioninstead. - The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in
IFoo. - Reference counting our proxies allows us to limit one memory allocation per Unique Handler
- We only construct a proxy for
Action<object>andActionhandlers -Action<T>handlers are added to the backing event object as normal. AddandRemovewhitelist onlyAction<object>andAction, and return null in all other cases, whichevent +=andevent -=handles gracefully.
The updated code with examples can be found as a DotNetFiddle here, and as a gist here.
$endgroup$
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
add a comment
|
$begingroup$
Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.
Thank you specifically for solutions by
dfhwze - using a single backing collection instead of one for each event
Henrik Hansen - using reference counting to avoid needing a proxyListfor each handler.
VisualMelon - separating proxy management logic as its own object.
I've compiled my own approach that pulls from each of these answers
A Utility for Managing the event proxies
I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.
As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.
My implementation of the Event Proxy Utility object is:
public struct EventProxyContainer<T>
private struct EventProxy
public Action<T> proxy;
public int count;
private Dictionary<object, EventProxy> handlerProxies;
public Action<T> Add(object handler) /* See Below */
public Action<T> Remove(object handler) /* See Below */
Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.
This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.
Add Implementation
public Action<T> Add(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count += 1;
handlerProxies[handler] = entry;
else
entry = new EventProxy() count = 1 ;
if(handler is Action<object>)
entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
else if (handler is Action)
entry.proxy = (v) => ((Action)handler).Invoke();
handlerProxies.Add(handler, entry);
return entry.proxy;
Remove implementation
Again we early out if the handler is Action<T>
public Action<T> Remove(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count -= 1;
if(entry.count == 0)
handlerProxies.Remove(handler);
else
handlerProxies[handler] = entry;
return entry.proxy;
Foo<T> Implementation
This really cleans up the Foo<T> implementation quite nicely:
public class Foo<T> : IFoo<T>
private EventProxyContainer<T> changeProxy;
public event Action<T> ChangeValue = delegate ;
public event Action<object> ChangeObject
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public event Action ChangeEmpty
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public void InvokeChange(T value)
ChangeValue.Invoke(value);
I like this approach because
- It still satisfies all the original requirements, and produces the same output as my original examples.
- It can be retrofitted in any case where you have an event of type
Action<T>where you also want event listeners to be able to use it asAction<object>andActioninstead. - The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in
IFoo. - Reference counting our proxies allows us to limit one memory allocation per Unique Handler
- We only construct a proxy for
Action<object>andActionhandlers -Action<T>handlers are added to the backing event object as normal. AddandRemovewhitelist onlyAction<object>andAction, and return null in all other cases, whichevent +=andevent -=handles gracefully.
The updated code with examples can be found as a DotNetFiddle here, and as a gist here.
$endgroup$
Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.
Thank you specifically for solutions by
dfhwze - using a single backing collection instead of one for each event
Henrik Hansen - using reference counting to avoid needing a proxyListfor each handler.
VisualMelon - separating proxy management logic as its own object.
I've compiled my own approach that pulls from each of these answers
A Utility for Managing the event proxies
I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.
As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.
My implementation of the Event Proxy Utility object is:
public struct EventProxyContainer<T>
private struct EventProxy
public Action<T> proxy;
public int count;
private Dictionary<object, EventProxy> handlerProxies;
public Action<T> Add(object handler) /* See Below */
public Action<T> Remove(object handler) /* See Below */
Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.
This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.
Add Implementation
public Action<T> Add(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count += 1;
handlerProxies[handler] = entry;
else
entry = new EventProxy() count = 1 ;
if(handler is Action<object>)
entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
else if (handler is Action)
entry.proxy = (v) => ((Action)handler).Invoke();
handlerProxies.Add(handler, entry);
return entry.proxy;
Remove implementation
Again we early out if the handler is Action<T>
public Action<T> Remove(object handler)
if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();
EventProxy entry;
if(handlerProxies.TryGetValue(handler, out entry))
entry.count -= 1;
if(entry.count == 0)
handlerProxies.Remove(handler);
else
handlerProxies[handler] = entry;
return entry.proxy;
Foo<T> Implementation
This really cleans up the Foo<T> implementation quite nicely:
public class Foo<T> : IFoo<T>
private EventProxyContainer<T> changeProxy;
public event Action<T> ChangeValue = delegate ;
public event Action<object> ChangeObject
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public event Action ChangeEmpty
add => ChangeValue += changeProxy.Add(value);
remove => ChangeValue -= changeProxy.Remove(value);
public void InvokeChange(T value)
ChangeValue.Invoke(value);
I like this approach because
- It still satisfies all the original requirements, and produces the same output as my original examples.
- It can be retrofitted in any case where you have an event of type
Action<T>where you also want event listeners to be able to use it asAction<object>andActioninstead. - The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in
IFoo. - Reference counting our proxies allows us to limit one memory allocation per Unique Handler
- We only construct a proxy for
Action<object>andActionhandlers -Action<T>handlers are added to the backing event object as normal. AddandRemovewhitelist onlyAction<object>andAction, and return null in all other cases, whichevent +=andevent -=handles gracefully.
The updated code with examples can be found as a DotNetFiddle here, and as a gist here.
edited Jun 2 at 10:41
answered Jun 2 at 7:53
JohannesJohannes
1976 bronze badges
1976 bronze badges
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
add a comment
|
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
1
1
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
Since Action<T> is your main signature, I can understand why you would convert the others to this one, leaving you no problems whatsoever on invocation. I can live with that :-p
$endgroup$
– dfhwze
Jun 2 at 8:05
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
I case you wonder why the downvote... I find you should have accepted one of the other answers instead of your own especially that they all very good.
$endgroup$
– t3chb0t
Jul 16 at 5:37
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
$begingroup$
@t3chb0t That is completely valid. If I could select multiple answers I would have done so. I chose to combine them and credit each of the original authors at the top of my answer as opposed to having to choose one. I felt each of them had something valid to contribute and not one of them was "better" than the rest.
$endgroup$
– Johannes
Jul 16 at 18:54
add a comment
|
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221457%2fmaking-an-actiont-event-also-usable-as-actionobject-and-action-in-c%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
You are very specific in what you don't want. But I do have a question about what you do want. You don't want to use a Action<Object>, but you do wrap all your event callers to Action<T>. Is this a requirement or can we use - let's say - 'object' as base class?
$endgroup$
– dfhwze
Jun 1 at 8:22
$begingroup$
For clarity (although I see you already added an answer): What I want is that IFoo<T> is usable exactly as specified, and that any implementation meets the requirements I listed, specifically that the examples I provide have identical output. Everything else is implementation details that do not matter within the context of this question.
$endgroup$
– Johannes
Jun 2 at 4:38