Formatting of hard to read try..catch..finally blocks?

Answer 1

Whitespace. As a bare minimum I always put one line of whitespace before every return statement and in between "doing stuff" and "creating variables" sections of code.

try
{
     MyService service = new Service();

     service.DoSomething();

     return something;

 }
catch (Exception ex)
{
     LogSomething();

     return somethingElse;

}
finally
{
     MarkAsComplete();
     service.Dispose();
}

much better.

Answer 2

Well, I think that's just fine. Some of this gets intot he curly brace placement debate. You could do this:

try {
  //
} catch(Exception ex) {
  //
} finally {
  //
}

I prefer what you have though. However, you might want to consider revising your code to only have one return statement. I find that is a little better design.

Answer 3

I format the code with the brackets on the same line:

try {
   MyService service = new Service();
   service.DoSomething();
   return something;
} catch (Exception ex) {
   LogSomething();
   return somethingElse;
} finally {
   MarkAsComplete();
   service.Dispose();
}

I prefer to add blank lines if I want more spacing. That also works as a separator between logical blocks of code.

Answer 4

Actually, this reads very well to me. If your actual code looks a lot like this, then I really wouldn't worry about it. It is VERY clear what is happening.

If your actual code is more complex, then consider breaking the blocks into well-named methods.

Answer 5

I think your formatting reads well too. My suggestion would be to only use the catch statement sparingly. Only use it when you actually need to catch something. Otherwise you can let other parts of the program handle the exception. The whole "fail early" concept.

try
{
    //do something that may throw an exception.
}
finally
{
    //handle clean up.
}

//let a method further down the stack handle the exception.

Answer 6

Personally, I tend to follow the preceding style in my code... It gives room to make comments, and shows the flow of my logic better.

I have a widescreen that I turn on it's side, so the whitespace allows the various columns to line up well and doesn't hurt me that much that because I have so much screen real-estate as it is...

try { // getting a service

     MyService service = new Service();
     service.DoSomething();

     return something;

}

catch (Exception ex) { // the fact that a service might be full/timedout

     LogSomething();

     return somethingElse;

} 

finally { // remove any resources the service may still hold.

     MarkAsComplete();
     service.Dispose();

}

Answer 7

You might think about containers (very smart factories) and advice (to handle all the messy details).

Dear Mr. Container Sir,
Whenever I request from you an instance object of the interface ISomething,
    please construct for me an instance of the concrete class SomethingImpl;
    in addition, please see to it (however you do it) that, whenever I call a
    method on this instance, it is wrapped within a complicated and messy try-
    catch-finally which logs exceptions and mark calls as completed. That way,
    all I have to do is write the business logic that goes into the SomethingImpl
    and I don't have to worry about all the messy infrastuctural details.
Sincerely,
Mr. Agile.

You might see this, in code, as:

//a class that knows how to take care of the messy infrastructure details
public class MyMessyInterceptor : IInterceptor {
    public void Intercept(IInvocation invocation) {
        //handle the messy details of continuing with the method-invocation,
        //but within a try-catch-finally that includes exception handling and
        //call logging.
    }
}

//a function that will configure a container (very smart factory)
public IContainer CreateContainer() {
    var builder = new ContainerBuilder();

    //tell the container-builder about the interceptor
    builder
        .Register(c => new MyMessyInterceptor())
        .Named("keep-my-code-clean")
    ;

    //tell the container what to do when you ask it for a ISomething
    builder
        .Register<SomethingImpl>()
        .As<ISomething>()
        .InterceptedBy("keep-my-code-clean")
    ;

    return builder.BuildContainer();
}

//some function out there in your code somewhere that needs to make a
//service call; there's hundreds of functions out there just like this
//in your code, and they all just got much simpler
public object GottaGoDoSomething() {
    //find the container
    var container = GetTheSingletonContainerObject();
    //ask for an instance of ISomething - it knows to provide a
    //SomethingImpl wrapped in an interceptor that takes care of all
    //the logging and exception handling
    var something = container.resolve<ISomething>();
    //call the big method
    return something.DoSomething();
    //magically (not really), the exception handling and logging are
    //already taken care of
}

Coming up with the interceptor class happens just once. Registering each interceptor and service class also happens just once. Setting up the container (very smart factory) is certainly complicated.

However, every place in your code that has to use the service object, and has to embed that use within complicated and messy infrastructure details such as exception handling and logging, just got very clean and very uncomplicated. There's only one CreateContainer, but there are hundreds of GottaGoDoSomethings, so that's a whole lot of easy at the cost of a little bit of complicated.

(Notes: The code example uses the Autofac container framework and the Castle interceptor framework. I am aware that this is an example of the service-location pattern, not the dependency-injection pattern, but the point was to illustrate interceptors and registering them with a container, not to illustrate dependency-injection.)

Answer 8

I too, like what you originally had. Physical lines in a .cs file don't cost you anything, and don't change your final output code. So use all you need in order to provide the best readability for you or your team.

In fact, you should actually try to use more lines than the 16 you show here when you code, by adding comments for yourself or others.

By adding

// a comment that says what's going on

frequently, you can better remind yourself what this Try.Catch is supposed to be doing when you go back to it after 6 months.

Answer 9

you can use a using block instead of an explicit Dispose(), or else you probability have to check for null before disposing it, using blocks does that for you. unfortunately it does increase nesting =/

try
{
     using(MyService service = new MyService()) 
     {
        service.DoSomething();
        return something;
     }
}
catch (SpecificException ex)
{
     LogSomething(ex);
     return somethingElse;
}
finally
{
     MarkAsComplete();
}

Answer 10

I always try and refactor out all of my try catch blocks and encapsulate them in their own method.

This always seems to make everything more readable, plus it's a good programming practice to make your methods only do one thing. Odds are that if you have code above and below your try-catch-finally statement, then you're doing more than one thing.