Saturday, July 12, 2008

Code reduction through generic methods

I've build and now maintain a webservice application at work. The application has around 40 webservice functions and earlier this week I had to dive into the application again because of some issues. It occurred to me how redundant all the code in the webservice functions was. I'm talking about the top-level code, the stuff in the *.asmx.cs files.

In this application the code there deals with input validation, authentication/authorization and sending back the results of the operation (success or error data). So most of the code in those ~40 functions is basically the same. The only thing that differs each time is the specific business logic that the function offers to the world, but that logic is nicely abstracted from the interface part of the application in the business logic part of the application so it's only a few lines of code in each top-level function to setup and call that specific handler. The main code of each function is stuff that each and every function needs to do. All copied and pasted. That can be done better.

I started off by creating a base class from which all webservices should inherit. This base class would provide the functions for input validation (each input object now implements an IValidating interface that is called so the object can basically validate itself) and authentication/authorization. This still left behind a large chunk of code (large chunk being relative here. It was no more then 18  lines of code, but for most functions that was still more then 50% of code in that function). The problem is that each separate webservice function returns a specific return type based on the function itself. A function called "CreateAccount" returns a "CreateAccountResult" object, a function called "SetOptions" returns a "SetOptionsResult" object, etc. So it's not possible to write a helper method, or a method in the base class like I could with validation code or the authentication/authorization code. But then I remembers generic methods.

I don't understand why I've never done these optimizations before. I've used generic methods years before in C++ and I know it was possible in C#. I just never thought about it. So with this new idea I mind I started coding.

We throw our own exceptions in the application. And we make a distinction between two kinds of errors: functional errors and technical errors. The main difference between the two being that function errors are logged as warnings and technical errors are logged as errors. Function errors come from business logic decisions (invalid input, creating an account that already exists, etc) and technical errors come from unexpected problems (SQL server unavailable, External webservice unavailable, etc). So all our own exceptions inherit from one of these two base exceptions. Also all exceptions have an error message and an error code.

The results we return in our webservice functions also inherit from a common base (the BaseResult class) that provides them with at least an error message and an error code (which get filled from the exception). With all that in mind I copied and changed the code that looked like:

1 string errorMessage = "CreateAccount error: " + e.Message; 2 if (e is TechnicalException) 3 { 4 TechnicalException technicalException = (TechnicalException)e; 5 ErrorLog.Error(errorMessage); 6 result = new CreateAccountResult(technicalException.ErrorCode, technicalException.Message); 7 } 8 else if (e is FunctionalException) 9 { 10 FunctionalException functionalException = (FunctionalException)e; 11 ErrorLog.Warn(errorMessage); 12 result = new CreateAccountResult(functionalException.ErrorCode, functionalException.Message); 13 } 14 else 15 { 16 ErrorLog.Error(errorMessage, e); 17 result = new CreateAccountResult(-1, "An unhandled exception occured: " + e.Message); 18 }

Into a function in the base class that looks like:

1 protected T HandleException<T>(Exception e, string functionName) where T : BaseResult, new() 2 { 3 T result = null; 4 string errorMessagePrefix = functionName + " "; 5 6 if(e is TechnicalException) 7 { 8 TechnicalException technicalException = (TechnicalException)e; 9 ErrorLog.Error(errorMessagePrefix + technicalException.Message); 10 result = new T { Code = technicalException.ErrorCode, Message = technicalException.Message }; 11 } 12 else if(e is FunctionalException) 13 { 14 FunctionalException functionalException = (FunctionalException)e; 15 ErrorLog.Warn(errorMessagePrefix + functionalException.LogErrorMessage); 16 result = new T { Code = functionalException.ErrorCode, Message = functionalException.Message }; 17 } 18 else 19 { 20 ErrorLog.Error(errorMessagePrefix + ": " + e.Message, e); 21 result = new T { Code = -1, Message = "An unhandled exception occured: " + e.Message }; 22 } 23 24 return result; 25 }

And replaced all code that is similar to the first code block with this one line:

1 result = HandleException<CreateAccountResult>(e, "CreateAccount");

A significant reduction as you can see. I definitely should have thought about this before.

No comments: