Agatha and a Unified Request Handler Theory

At my last job we were using the Agatha library to simplify requests and responses through our WCF service. All in all it was pretty cool. But there was one thing I didn't like. Every request/response pair needed its own handler class.

Now, normally this makes sense. However, we were dealing with a growing enterprise domain. We're talking about potentially hundreds upon hundreds of handlers. From a code organizational perspective, it made more sense to group our domain methods into categories. Thus were created processor classes. Each subset of the overall domain logic (product ordering, reporting, security, etc.) would have a processor class (or namespace, with further divided classes) with all the methods in there.

This left us with a growing namespace of handler classes, each of which was nearly identical. We had our base handler which inherited from Agatha's handler and did our common stuff (error trapping, unit of work encapsulation, etc.) that every service call needs. It included an abstract method that its Handle method would call. Then, for every request/response pair in the domain, a handler was written which implemented this abstract method and passed control along to the proper processor method.

This means that we were going to have hundreds upon hundreds (maybe more) of identical handlers, all doing precisely the same thing. That's a lot of repetition in the code, and it's an easy point for a new junior dev to get hung up on when he's not used to the system. He shouldn't be expected to "just know" that he has to implement this intuitively useless class. All he should have to do is create his request/response types and his domain logic method. The base handler should take care of directing traffic.

It was a dream born of a desire to simplify and to make the programming as plug-and-play as possible for new junior devs on the team. And this dream has now produced a prototype. Keep in mind that this is not finished. Also, it's SLOW. But it works. Hopefully I can make enhancements to it (feel free to suggest any) or even just re-implement the idea in an entirely different and better way. This is just a proof of concept:

public class MyBaseRequestHandler : IRequestHandler
  where TREQUEST : MyBaseRequest
{
  public Response Handle(TREQUEST request)
  {
    try
    {
      // TODO: Error-check the request

      // Get the assembly for the domain code
      // At my last job they had an empty "marker" class.  Made sense.
      var assembly = Assembly.GetAssembly(typeof(DomainAssemblyMarker));

      // Create a response based on the request type
      // I use a custom attribute called "ResponseTypeAttribute" to
      // store the response type for any given request type at the
      // class level.
      var response = Activator.CreateInstance(
        ((ResponseTypeAttribute)
          request.GetType().GetCustomAttributes(
                              typeof(ResponseTypeAttribute),
                              false)[0]
        ).TypeName
      );

      try
      {
        // Do the process for this request
        // The convention is that there should be exactly ONE method
        // in the processors which takes ONE argument (the request)
        // and returns the response.
        // TODO: Implement a true "unit of work" here
        foreach (var type in assembly.GetTypes())
          if (type.IsClass)
            foreach (var method in type.GetMethods())
              if (method.GetParameters().Length == 1)
                if (method.GetParameters()[0].ParameterType == typeof(TREQUEST))
                {
                  // Get an instance from the IoC and invoke the method
                  var obj = ObjectFactory.GetInstance(type);
                  return (Response)method.Invoke(obj, new object[] { request });
                }

        // Couldn't find the right method, return a failed response
        // The .Failed extension (and .Success) was something they
        // implemented at my old job.  Kind of got me hooked on
        // extensions.  So expect more in the future.
        return ((MyBaseResponse)response).Failed(
          new Message
          {
            DisplayMessage = "Service Error",
            ErrorMessage = "Unable to find processor method for "
                     + request.GetType().ToString(),
            MessageType = MessageType.Error
          }
        );
      }
      catch (Exception ex)
      {
        // Caught an exception trying to find and/or invoke the handler method
        return ((MyBaseResponse)response).Failed(ex);
      }
    }
    catch (Exception ex)
    {
      // Caught an exception trying to create a properly typed reponse based on
      // the request type.  Just return a base response with the failed message,
      // even though the client probably won't like the type.
      return new MyBaseResponse().Failed(ex);
    }
  }

  public Response Handle(Request request)
  {
    return Handle((TREQUEST)request);
  }

  public void Dispose()
  {
    // TODO: Handle disposal
  }

  public Response CreateDefaultResponse()
  {
    return new MyBaseResponse();
  }
}

Ya, you can see the slow part. It's kind of obvious. So the main improvement to be made here is in finding the method to invoke. Hey, at least it's properly asking for the object from the IoC container (StructureMap), right? I'm thinking of doing more with custom attributes (an area that's somewhat new to me) to assist in that method discovery process, similar to what I did with the request/response discovery (which I'd also like to improve a bit if possible).

Also, before I forget, I had to tell the StructureMap configuration that all IRequestHandlers should use MyBaseRequestHandler:

For(typeof(IRequestHandler<>)).Use(typeof(MyBaseRequestHandler<>));

You'll also notice that the last catch block in the Handle method returns a not-properly-typed response. I can confirm that the client does not like this. When I try to Get<>() a specific response from the dispatcher it blows up saying the sequence contains no elements. Ya, that response isn't in there. MyBaseResponse is. So hopefully I can find a way to clean this up as well.

I'd love some input on this, since as I said it's just an early prototype.