9
\$\begingroup\$

I'm trying to implement generic queries and handlers so that I can make requests with simple syntax, like this:

var query = new HelloQuery("hi");
var result = processor.Process(query);

This post explains a brilliant technique for accomplishing this, but it uses reflection and dynamic to resolve handlers for queries. I came up with an alternative using generics.

The central idea is the IQuery:

public interface IQuery<TSelf, TResult>
    where TSelf : IQuery<TSelf, TResult>
{
}

One such implementation could be:

public class HelloQuery : IQuery<HelloQuery, string>
{
    public HelloQuery(string message)
    {
        this.message = message;
    }

    private readonly string message;
    public string Message
    {
        get { return this.message; }
    }
}

The TSelf type argument is the key difference between my technique and the other post's technique. It allows us to avoid reflection in the HandlerResolver:

public interface IHandlerResolver
{
    IQueryHandler<TQuery, TResult> GetHandlerFor<TQuery, TResult>()
        where TQuery: IQuery<TQuery, TResult>;
}

public class HandlerResolver : IHandlerResolver
{
    private readonly IContainer container;

    public HandlerResolver(IContainer container)
    {
        this.container = container;
    }

    public IQueryHandler<TQuery, TResult> GetHandlerFor<TQuery, TResult>()
        where TQuery : IQuery<TQuery, TResult>
    {
        var handler = this.container.Get<IQueryHandler<TQuery, TResult>>();
        return handler;
    }
}

You would register handlers at the composition root. Here's the interface for a handler, and an implementation for HelloQuery:

public interface IQueryHandler<TQuery, TResult>
    where TQuery : IQuery<TQuery, TResult>
{
    TResult Handle(TQuery request);
}

public class HelloQueryHandler : IQueryHandler<HelloQuery, string>
{
    public string Handle(HelloQuery request)
    {
        var response = string.Format("Well \"{0}\" yourself!", request.Message);
        return response;
    }
}

And finally, the actual query processor:

public interface IQueryProcessor
{
    TResult Process<TQuery, TResult>(IQuery<TQuery, TResult> request)
        where TQuery : IQuery<TQuery, TResult>;
}

public class QueryProcessor : IQueryProcessor
{
    private readonly IHandlerResolver handlerResolver;

    public QueryProcessor(IHandlerResolver handlerResolver)
    {
        this.handlerResolver = handlerResolver;
    }

    public TResult Process<TQuery, TResult>(IQuery<TQuery, TResult> request)
        where TQuery : IQuery<TQuery, TResult>
    {
        var handler = this.handlerResolver.GetHandlerFor<TQuery, TResult>();
        var response = handler.Handle((TQuery)request);
        return response;
    }
}

The interfaces would be simpler without the queries' TSelf type argument, but then the C# compiler can't infer the types, and you end up having to spell out the full Process() call like processor.Process<HelloQuery, string>(query). I'm trying to avoid this ugliness.

My question is: Is the TSelf type argument a code smell? Is there a better alternative that doesn't require falling back on reflection?

\$\endgroup\$
  • \$\begingroup\$ I'd be interested to see how you're going to use your queries in a more complex example. I think you did a good job at offloading the ugliness in HandlerResolver \$\endgroup\$ – mariosangiorgio Feb 2 '15 at 21:18
  • \$\begingroup\$ Loved this solution. Avoiding reflection, dynamic and type casting is crucial for performance, because my controllers spend much time executing commands and queries. \$\endgroup\$ – Jone Polvora May 5 '15 at 20:28
  • \$\begingroup\$ @JonePolvora You said it avoided casting, but there's a cast in handler.Handle((TQuery)request); I don't like the dynamic of the original post, but the OP has written a lot of code only to then through a cast in at the last second!! \$\endgroup\$ – matt_lethargic Jul 18 '17 at 8:52
6
\$\begingroup\$

One objection I could raise is that your generic type constraint doesn't guarantee what you think it might guarantee. Eg. you could have:

// TSelf is actually not T of Self
public class HelloQuery : IQuery<GoodbyeQuery, string> 
{
}

// This one is fine
public class GoodbyeQuery: IQuery<GoodbyeQuery, string> 
{
}

In other words, TSelf doesn't actually have to be the T of the class itself. This is not horrible, especially since you seem to only be using the interface as metadata, but it can still lead to confusing results during dependency injection if you've accidentally screwed up a class definition.

However, as long as you're not a masochist and don't declare nonsensical types like this, there is nothing wrong with this. This is an established design pattern known as the curiously recurring template pattern.

\$\endgroup\$
  • \$\begingroup\$ Thanks, Asad. I decided against using my implementation above, partly because of your answer. I came up with a compromise implementation, which you can see here if you're curious. \$\endgroup\$ – Bryan Feb 12 '15 at 19:12
2
\$\begingroup\$

It's unrelated to your primary concern, but this pattern shows up in most of the code you've shared with us.

public class HelloQuery : IQuery<HelloQuery, string>
{
    public HelloQuery(string message)
    {
        this.message = message;
    }

    private readonly string message;
    public string Message
    {
        get { return this.message; }
    }
}

It may be a bit subjective, but this feels like a lot of wasted effort. Properties like this can be defined with a private set which will remove a lot of boiler plate.

public class HelloQuery : IQuery<HelloQuery, string>
{
    public HelloQuery(string message)
    {
        this.Message = message;
    }

    public string Message { get; private  set; }
}

To the best of my knowledge, this behaves just the same as your original code.

What's the benefit? It's less lines of code to maintain and as such it lowers the opportunity for future bugs to creep in as changes are made to the code.

Or, if you're using C# 6, you can use a "getter-only" auto-property. Credit to @DanLyons.

public class HelloQuery : IQuery<HelloQuery, string>
{
    public string Message { get; }

    public HelloQuery(string message)
    {
        Message = message;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Those two snippets are not equal. In the second snippet HelloQuery can assign to Message as many times as it wants to. In the first snippet, HelloQuery can only assign to message once. \$\endgroup\$ – Nick Udell Feb 3 '15 at 12:15
  • \$\begingroup\$ @NickUdell yes, but it's only being set in the constructor and once in the constructor. In that sense, they are equivalent are they not? If the code was assigning values multiple times to the same field in the constructor, I would consider that a smell in and of itself. Am I misunderstanding something here? \$\endgroup\$ – RubberDuck Feb 3 '15 at 12:18
  • 1
    \$\begingroup\$ very true, just thought it was worth mentioning for anybody that came along. \$\endgroup\$ – Nick Udell Feb 3 '15 at 12:20
  • \$\begingroup\$ It was worth mentioning @NickUdell because I myself didn't understand the subtle differences, although I knew there were some. \$\endgroup\$ – RubberDuck Feb 3 '15 at 12:23
  • 3
    \$\begingroup\$ In C# 6, you can use getter-only auto-properties to obtain the equivalent read-only behavior. \$\endgroup\$ – Dan Lyons Feb 3 '15 at 18:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.