Skip to content
This repository has been archived by the owner on Jan 28, 2025. It is now read-only.

Check for null when resolving handlers #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cpx86
Copy link

@cpx86 cpx86 commented Oct 24, 2017

Fixes #8

@danielwertheim
Copy link
Owner

Looking at the code I got some ideas that I would like to run by you. What do you think of doing something like this:

public static class MessageHandlerCreatorExtensions
{
    public static object RequiredHandlerInvoke(
        this MessageHandlerCreator creator,
        Type messageHandlerType,
        MessageEnvelope envelope)
            => creator(messageHandlerType, envelope)
            ?? throw new InvalidOperationException($"Message handler creator could not create a message handler of type {messageHandlerType.FullName}.");
}

Then if someone wants to write their own dispatcher or router they can easily use the same logic. This would be used like this:

var handler = _messageHandlerCreator.RequiredHandlerInvoke(action.HandlerType, envelope);

Could then potentially not do any change of existing routers and dispatchers and leave it up the the user:

public static class MessageHandlerCreators
{
    public static MessageHandlerCreator NotAcceptingNullHandlers(MessageHandlerCreator creator)
        => creator.RequiredHandlerInvoke;
}

which then could be used:

UnitUnderTest = new AsyncDispatcher(
    MessageHandlerCreators.NotAcceptingNullHandlers((t, e) => Activator.CreateInstance(t)), routes);

I guess the footprint of pre-creating the handlers isn't to large, how-ever, it will be done once per message of the same type routed. So for n > 1 it's really unnecessary, because if it could be created the first time... And yes, then the check is also unnecessary, but perhaps fair. Ideally the user should pick the strategy. I mean, if the creator returns null they have messed up. Perhaps offer something that can be used when they bootstrap everything. Run their creator through all the routes to ensure it's satisfied?

@danielwertheim
Copy link
Owner

@cpx86 what's your opinion on the above?

@cpx86
Copy link
Author

cpx86 commented Oct 30, 2017

@danielwertheim Sorry, was a bit swamped lately..

I like the idea of making it less intrusive on the implementors. Would it be possible to take it one step further and actually wrap the MessageHandlerCreator before passing it to the underlying router/dispatcher? That way the routers wouldn't even need to change, and new routers wouldn't need to know about this behavior. Not sure if it would make any difference on performance or number of allocations though - I'm thinking that pre-wrapping it could affect the ability to inline it - that's something that could be worth profiling.

@danielwertheim
Copy link
Owner

@cpx86 I might be misunderstanding now, but isn't that what my last "sample" does?

UnitUnderTest = new AsyncDispatcher(
    MessageHandlerCreators.NotAcceptingNullHandlers((t, e) => Activator.CreateInstance(t)), routes);

Sure it could be a type or something, but the idea of the sample is to wrap the passed creator with a behavior of not accepting null resolves. That way, it's up to the user when bootstrapping.

The other code can very well be used by "authors" to enforce it in their specific routers/dispatchers.

@cpx86
Copy link
Author

cpx86 commented Oct 30, 2017

Ah my bad, I mixed something up in my head :)

Letting it be up to the user makes sense, I agree. Although the question then is, does it even need to be provided by Routemeister? In that case the user could just create their own extension if they want to simplify handling null-resolved handlers (since it is anyway dependent on what factory you use).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants