Question

I'm trying to figure out the "best" way execute a (close) statement in my sql data access layer methods.

I was wondering which of the two ways is considered more correct (thanks):

Option #1

public void dbOperation( ... )
{
    try
    {
        _cmd.Open();        
        _cmd.Execute();
    }
    catch (Exception ex)
    {
        throw ex;       
    }
    finally
    {
        _cmd.Close()
    }   
}

Option #2

public void dbOperation( ... )
{
    try
    {
        _cmd.Open();        
        _cmd.Execute();
    }
    catch (Exception ex)
    {
        _cmd.Close()    
        throw ex;   
    }

    _cmd.Close();
}
Was it helpful?

Solution

Neither is correct. You shouldn't have a catch clause just to re-throw the exception again, clearing out its stack trace, and doing nothing productive, which is what your first option does.

You should just be closing in the finally:

try
{
    _cmd.Open();        
    _cmd.Execute();
}
finally
{
    _cmd.Close()
} 

Your second snippet has the same problem with it, since you're improperly re-throwing the exception.

The best option would be to use a using, which is just a syntactic sugar for a try/finally without a catch:

using(var command = ...)
{
    command.Open();        
    command.Execute();
}

This has the added benefit of also ensuring that the scope of the command is exactly the same as when it's valid to use it. A try/finally block requires the command to be a valid identifier after it has been disposed of.

OTHER TIPS

Option #1 is the only correct one of the two. In fact, you can get the equivalent of option #1 using less code:

try
{
    _cmd.Open();        
    _cmd.Execute();
}
finally
{
    _cmd.Close()
}

If an exception is thrown, there's no need to have a catch block to just re-throw it. If you want to do something in the catch block, such as logging the exception, be sure to do this:

try
{
    _cmd.Open();        
    _cmd.Execute();
}
catch (Exception ex)
{
    logException(ex);
    throw;  //Just say throw, not throw ex, to preserve the original stack trace
}
finally
{
    _cmd.Close()
} 

Option #1. If you use option2 you won't execute cmd.close unless it throws an exception

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top