2
\$\begingroup\$

I'm building an http handler at work which has about 6 methods and I'm trying to figure out what design pattern will work the best for my needs:

What's done already (Only an example to make this more clear, methods are not real, but the main concept is there):

#region interfaces
public interface IDateRange
{
    DateTime StartDate { get; set; }
    DateTime EndDate { get; set; }
}

public interface ISomeMoreParams
{
    int Param1 { get; set; }
    int Param2 { get; set; }
}

public interface IBaseParams
{
    string UserName { get; set; }
    string CompanyName { get; set; }
}

public interface IClassMethod : IBaseParams
{
    void ExecuteLogic(HttpContext context);
}
#endregion

#region FactoryClass
public static class ClassMethodFactory
{
    public static IClassMethod GetInstance(string methodName)
    {
        switch(methodName)
        {
            case "MethodA":
                return new MethodA();
            case "MethodB":
                return new MethodB();
            case "MethodC":
                return new MethodC();
        }

        throw new NotImplementedException(string.Format("{0} is not implemented", methodName));
    }
}
#endregion

#region Class methods
public class MethodA : IClassMethod, IDateRange
{
    string UserName { get; set; }
    string CompanyName { get; set; }
    DateTime StartDate { get; set; }
    DateTime EndDate { get; set; }

    public void ExecuteLogic(HttpContext context)
    {
        // Do MethodA
    }
}

public class MethodB : IClassMethod, ISomeMoreParams
{
    string UserName { get; set; }
    string CompanyName { get; set; }
    int Param1 { get; set; }
    int Param2 { get; set; }

    public void ExecuteLogic(HttpContext context)
    {
        // Do MethodB
    }
}

public class MethodC : IClassMethod, IDateRange, ISomeMoreParams
{
    string UserName { get; set; }
    string CompanyName { get; set; }
    DateTime StartDate { get; set; }
    DateTime EndDate { get; set; }
    int Param1 { get; set; }
    int Param2 { get; set; }

    public void ExecuteLogic(HttpContext context)
    {
        // Do MethodC
    }
}
#endregion

#region HttpHandler
public IClassMethod method;

public void ProcessRequest(HttpContext context)
{
   ProcessParams();

   ExecuteLogic();
}

public ProcessParams()
{
    var qs = HttpContext.Current.Request.QueryString;

    method = ClassMethodFactory.GetInstance(qs["action"]);

    if(method is IDateRange)
    {
        (method as IDateRange).StartDate = DateTime.Parse(qs["startDate"]);
        (method as IDateRange).EndDate = DateTime.Parse(qs["endDate"]);
    }

    if(method is ISomeMoreParams
    {
        (method as ISomeMoreParams).Param1 = int.Parse(qs["param1"]);
        (method as ISomeMoreParams).Param2 = int.Parse(qs["param2"]);
    }

    if(method is IBaseParams)
    {
        (method as IBaseParams).UserName = qs["UserName"];
        (method as IBaseParams).CompanyName = qs["CompanyName"];
    }
}

public ExecuteLogic()
{
    method.ExecuteLogic();
}
#endregion

Now, the only part the I have a problem with, is the casting each time I want to see if the class method implements some parameter logic. I wish it would be more organized.

My thoughts: 1. Create for each parameter interface a parameter resolver class which would look like this:

#region Resolvers
public class DateRangeResolver
{
    public void Resolve(IDateRange dateRangeClass)
    {
        var qs = HttpContext.Current.Request.QueryString;

        dateRangeClass.StartDate = DateTime.Parse(qs["startDate"]);
        dateRangeClass.EndDate = DateTime.Parse(qs["endDate"]);
    }
}

public class SomeMoreParamsResolver
{
    public void Resolve(ISomeMoreParams someMoreParamsClass)
    {
        var qs = HttpContext.Current.Request.QueryString;

        someMoreParamsClass.Param1 = int.Parse(qs["param1"]);
        someMoreParamsClass.Param2 = int.Parse(qs["param2"]);
    }
}

public class BaseParamsResolver
{
    public void Resolve(IBaseParams baseParamsResolver)
    {
        // Do resolve
    }
}
#endregion

Then I change the code to fit the new approach:

#region interfaces

// ...

public interface IClassMethod : IBaseParams
{
    void ProcessParams();

    void ExecuteLogic();
}

// ...

#endregion

#region Class methods

// ...

public class MethodA : IClassMethod, IDateRange
{
    // params

    public void ProcessParams()
    {
        new BaseParamsResolver().Resolve(this);
        new DateRangeResolver().Resolve(this);
    }

    public void ExecuteLogic()
    {
        // Do MethodA
    }
}

public class MethodB : IClassMethod, ISomeMoreParams
{
    // params

    public void ProcessParams()
    {
        new BaseParamsResolver().Resolve(this);
        new SomeMoreParams().Resolve(this);
    }

    public void ExecuteLogic()
    {
        // Do MethodB
    }
}

public class MethodC : IClassMethod, IDateRange, ISomeMoreParams
{
    // params

    public void ProcessParams()
    {
        new BaseParamsResolver().Resolve(this);
        new DateRangeResolver().Resolve(this);
        new SomeMoreParams().Resolve(this);
    }

    public void ExecuteLogic(HttpContext context)
    {
        // Do MethodC
    }
}

// ...

#endregion

#region HttpHandler

// ...

public void ProcessParams()
{
    method.ProcessParams();
}

//...

#endregion

Now, my problem is that if I don't call the right resolvers on the ProcessParams of each class method, I would get an exception at runtime and not on debug, which means, I need to consider a new approach or change the existing approach to do so.

I had another idea, the idea is to add to each parameter interface its own resolver so when a class method implements the param interface it would also need to define the resolver for the interface.

This idea, although sounds better, still doesn't enforce the programmer to actually call these resolvers when executing the ProcessParams() function.

Now, after this long long explanation... is there a better way?

\$\endgroup\$
  • \$\begingroup\$ “is there a better way?” Not reinventing the wheel and using something like ASP.NET Web API? \$\endgroup\$ – svick Dec 14 '13 at 23:26
  • \$\begingroup\$ And could you explain why wouldn't you get an exception on debug? \$\endgroup\$ – svick Dec 14 '13 at 23:35
  • \$\begingroup\$ svick, what do you mean? How will it let you do this different? \$\endgroup\$ – Imri Barr Dec 15 '13 at 16:03
  • \$\begingroup\$ svick, the exception would be only on run time since when debugging, the debugger doesn't know if those parameters have values and in the ExecuteLogic function on each class method, it would try to use these parameters but they would be null or default. \$\endgroup\$ – Imri Barr Dec 15 '13 at 16:05
  • \$\begingroup\$ When you say “debugger”, do you mean “compiler”? Because when you're debugging, the debugger does know what values do the parameters have right now. \$\endgroup\$ – svick Dec 15 '13 at 16:29
1
\$\begingroup\$

Let me admit up front I don't know a lot about implementing web handlers and the various frameworks, so this answer ignores a probably very legitimate point about reusing the wheels that are already out there. Instead this answer addresses how the code is factored.

Your third block of code is definitely an improvement on the first, but I would still say you are using way too many abstraction layers, resulting in way too much code for a simple task. You should let each class drive reading the querystring params. If you find there's a lot of shared work, factor out helper methods. I would probably avoid the extra classes for the level of complexity in your sample, opting instead for methods that call helpers. After all, if they're not related, why are they on the same HttpHandler?

public void ProcessRequest(HttpContext context)
{
    var qs = context.Request.QueryString;
    switch(qs["action"])
    {
        case "MethodA": return MethodA(context);
        case "MethodB": return MethodB(context);
        case "MethodC": return MethodC(context);
    }
    throw new NotImplementedException(string.Format("{0} is not implemented", qs["action"]));
}

void MethodA(HttpContext context)
{
    var qs = context.Request.QueryString;
    ResolveIdentity(qs, out UserName, out CompanyName); // straightforward helper method
    ResolveDates(qs, out StartDate, out EndDate);       // ditto; you can write them

    // do Method A logic
}

As for the true fact that you can't enforce that a new method will call the resolvers, don't worry about it. This isn't some corner case; it's a core need. It's not like it'll work right most of the time and then fail spectacularly when the moon is full; it will always fail. Rather than worrying about if it's possible to do the wrong thing, focus on making it easy to do the right thing.

As a side note, if (obj is interface) { (obj as interface) ...; } is an antipattern because it can pay casting costs multiple times. You should typically either check is then cast directly ((interface)obj), or store the results of as once and check it for null; don't mix the two.

\$\endgroup\$

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.