I've got a number of WebService methods that all include some very boilerplate code of wrapping the actual work in a try/catch/finally and performing the same tasks in the catch/finally. So as a way to encapsulate all of the shared catch/finally stuff I wrote a simple generic.
This works and really eliminates a bunch of repetitive code but feels klunky, and the syntax is very obtuse. Every time I come back to this my brain gets twisted trying to figure it out (a clear sign it is not a good design). I'm looking for feedback on whether this is a crazy thing to do, and if there is a better way to approach it.
Here is my template:
public delegate T2 RestfulServiceRequest<T1, T2>(T1 req);
static class RestfulService
{
public static T2 ExceptionHandler<T1, T2>(RestfulServiceRequest<T1, T2> serviceCall, T1 req)
{
if (req == null)
throw new BadRequestException(new ArgumentNullException("Invalid or missing request object"));
try
{
return serviceCall(req);
}
catch (RestfulException e)
{
// log it and rethrow
Logger.Write(e);
throw;
}
catch (Exception e)
{
Logger.Error(e);
// wrap in a consistent exception for propagation back to caller
throw new InternalServerException(e);
}
finally
{
Logger.Debug("Complete");
}
}
}
}
And here is a usage of it:
public class Initialization : IInitialization
{
// MyMethod thas uses the template
public ApplianceInitResp CreateApplianceServer(ApplianceInitReq req)
{
return RestfulService.ExceptionHandler<ApplianceInitReq, ApplianceInitResp>(delegate(ApplianceInitReq x)
{
// do some work
return new ApplianceInitResp();
}, req);
}
}
}
-
I would agree that this feels a bit clunky to accomplish, however, I am not right away seeing the "pefect" way to re-work this to minimize duplication.
THe only thing I'm thinking is that if you can somehow use interfaces or break it up a bit, I'm not sure exactly how I'd do it, but I can say that for on going changes I don't think I would leave this, at least not without really good documentation.
-
One thing that will make it cleaner is to define interfaces that your request/response objects implement. Then you can get rid of the generics in favor of the interfaces. Note, too, the name change which I think is more descriptive of what you are really trying to do.
public interface IServiceResponse { ... } public class ApplianceInitResp : IServiceResponse { ... } public interface IServiceRequest { ... } public class ApplianceInitReq : IServiceRequest { ... } public delegate IServiceResponse RestfulServiceRequest( IServiceRequest req ); static class RestfulService { public static IServiceResponse Invoke( RestfulServiceRequest serviceCall, IServiceRequest req) { if (req == null) throw new BadRequestException( ...inner-exception... ); try { return serviceCall(req); } catch (RestfulException e) { Logger.Write(e); throw; } catch (Exception e) { Logger.Error(e); throw new InternalServerException(e); } finally { Logger.Debug("Complete"); } } } public class Initialization : IInitialization { // MyMethod thas uses the template public ApplianceInitResp CreateApplianceServer(ApplianceInitReq req) { return RestfulService.Invoke( delegate(ApplianceInitReq x) { // do some work return new ApplianceInitResp(); }, req ); } }
BrettRobi : I like this solution as it is much easier to understand. The only drawback I see is the requirement that all service operations use args that derive from the interface. Not a big deal unless I need to return native types. -
I would suggest you to look for frameworks that offer AOP functionality (like Spring.NET, Unity). Those will help you to reduce your CreateApplianceServer() call to mere
public ApplianceInitResp CreateApplianceServer(ApplianceInitReq req) { // Do some work return new ApplianceInitResp(); }
by taking care of entrance/exit and exception logging with aspects. Probably, if you have some common parameters, you could plug the the argument validation into the aspect as well.
Of course, there would be a learning curve tax, but, trust me, result worth it. You will dramatically reduce amount of boilerplate code in your methods.
-
I think what you're doing is fine. This kind of coding is common in scripting languages -- but then it's less verbose because they're dynamically typed. And it's also common in functional languages -- but then it's less verbose because they have such good type inference! In C# you're somewhat hampered by the current limitations of the type inference system, but that will get better in version 4.
The alternative is to use a proliferation of interfaces or classes to encapsulate your work. As you do this, you have fewer type names at the point of invocation, but more code overall.
-
I would change
public static T2 ExceptionHandler<T1, T2>(RestfulServiceRequest<T1, T2> serviceCall, T1 req)
to
public static T2 Invoke<T1, T2>( this T1 req, RestfulServiceRequest<T1, T2> serviceCall)
This will change the call to
public class Initialization :IInitialization { public ApplianceInitResp CreateApplianceServer( ApplianceInitReq req ) { return req.Invoke( r => { // do some work return new ApplianceInitResp(); }); } }
BrettRobi : I have to admit that I do not understand how 'this T1 req' works. How does the compiler figure out to use the correct generic?
0 comments:
Post a Comment