Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

throw exception in the try block rather than catch block?

I've inherited code in our project which looks like this. It's a method in a class.

protected override bool Load()
{
    DataAccess.SomeEntity record;

    try
    {
        record = _repository.Get(t => t.ID.Equals(ID));

        if (record == null)
        {
            throw new InvalidOperationException("failed to initialize the object.");
        }
        else
        {
            this.ID = record.ID;
            // this.OtherProperty = record.SomeProperty;
            // etc
        } 
    }
    catch (Exception)
    {
        throw;
    }

    return true;
}

If I then call this Load method from my UI layer, I'd probably want to have a try catch block to catch any exception caused by the failure to Load the instance, e.g. InvalidOperationException, but the above code feels wrong to me.

Won't the InvalidOperationException be swallowed by the catch statement? that catch statement will also catch potential problems with _repository.Get, as well as potential problems with the setting of properties if the record is valid.

I thought I should perhaps restructure it by adding more try catch statements to handle the Get operation and property setting operations separately, or add more catch blocks handling different exceptions, but I asked a colleague, and he suggested that the try catch is irrelevant in this case, and should be removed completely, leaving it like:

protected override bool Load()
{
    DataAccess.SomeEntity record;

    record = _repository.Get(t => t.ID.Equals(ID));

    if (record == null)
    {
        throw new InvalidOperationException("failed to initialize the object.");
    }
    else
    {
        this.ID = record.ID;
        // this.OtherProperty = record.SomeProperty;
        // etc
    } 

    return true;
}

I'd like some second opinions, I've only just started taking an interest in exception handling, so I'd like to make sure I am doing it the right way according to best practices.

like image 883
Andrew Johns Avatar asked Aug 05 '10 11:08

Andrew Johns


1 Answers

When you do this:

catch (Exception)
{
    throw;
}

You are essentially not handling the exception. That does not however mean you are ignoring it. The throw statement will propagate the exception up the stack. For the sake of clean readable code your final example is much better.

like image 85
ChaosPandion Avatar answered Sep 18 '22 03:09

ChaosPandion