I develop/maintain an in-house library in a growing company with about 12 other developers that write code. This library is mostly used for scripting game behavior and we have many different projects that make use of it.
This library implements simple, typed, event-aggregation in a static class EventBus
.
It has the expected methods for subscribing and unsubscribing event-handlers:
Subcribe<TEvent>(Action<TEvent> handler)
and
Unsubscribe<TEvent>(Action<TEvent> handler)
My conundrum is a 'convenience method' I've added: SubscribeUntilTrue<TEvent>(Func<TEvent, bool> handler
), which is allows creating (anonymous) event-handlers which can decide if they wish to receive further event-messages by returning a boolean value. Essentially, they will be unsubscribed after some condition is fulfilled.
Example:
EventBus.SubscribeUntilTrue<DialogEvent.End>(e =>
{
if (e.Dialog.Name == "MyDialog")
{
DoSomething();
return true;
}
return false;
});
The goal of having this method is to allow the developer using it to signal intent more clearly, reduce boilerplate and avoid declaring methods that are called only once. Below are two examples of code that do the same thing as the snippet above in a way which I think is less readable/intuitive/maintainable:
First Example: Using a named method, which...
EventBus.Subscribe<DialogEvent.End>(EndHandler);
...is declared somewhere else.
private void EndHandler(DialogEvent.End e)
{
if (e.Dialog.Name == "MyDialog")
{
DoSomething();
EventBus.Unsubscribe<DialogEvent.End>(EndHandler);
}
}
This is not ideal because the intent can only be communicated through the name of the handler-method at the point of subscription and it is not clear if there are any other callers to the handler. I suspect that in many cases the name can easily become very verbose and or inaccurate as the typical code inside such a handler is also likely to change often while in development while also being short.
Second Example: A developer avoiding the declaration of a 'once-called' method might use a self-unsubscribing delegate/anonymous method which is a bit messy/ugly:
Action<DialogEvent.End> endHandler = null;
endHandler = e =>
{
if (e.Dialog.Name == "MyDialog")
{
DoSomething();
EventBus.Unsubscribe(endHandler);
}
};
EventBus.Subscribe(endHandler);
Developers who are not familiar with this pattern can easily become confused.
I am not sure the name SubscribeUntilTrue
is adequate for people to intuitively understand what the method does and what meaning it assigns the required return value of the handler.
It is generally considered to be a smell to include a condition(al) in the name of a method, like SubscribeUntilTrue. It was initially named just SubscribeUntil but I had to explain to a fellow developer what it does and he suggested to add the True part.
I've had an idea to change the signature of the method (or add an overload) so that the handler-function's return value is a sealed enum-like struct with two static properties that serve as 'constants' which would denote the intended usage more clearly:
EventBus.SubscribeUntil<DialogEvent.End>(e =>
{
if (e.Dialog.Name == "MyDialog")
{
DoSomething();
return EventHandlerResult.Unsubscribe;
}
return EventHandlerResult.KeepSubscribed;
});
This 'enum-like' struct with only two possible values has implicit conversions to/from bool so that switching between which overload is used is more convenient.
public struct EventHandlerResult
{
private readonly bool shouldUnsubscribe;
public static readonly EventHandlerResult Unsubscribe = true;
public static readonly EventHandlerResult KeepSubscribed = false;
private EventHandlerResult(bool shouldUnsubscribe)
{
this.shouldUnsubscribe = shouldUnsubscribe;
}
public static implicit operator bool(EventHandlerResult state)
{
return state.shouldUnsubscribe;
}
public static implicit operator EventHandlerResult(bool value)
{
return new EventHandlerResult(value);
}
}
Though this has some obvious issues like; If I change the signature it would force consumers to change the signature of handlers they've already written. I could keep both versions though.
Anyone out there have any idea of a better name for this method? Also is the enum-like struct a good/bad approach to guide/force the clarity/intent of code written consumers of the library?