CODEWITHSUNDEEP
c#.nettry-catch
Nate
Nate

Reputation: 30646

Is it bad to use nested Try..Catch blocks like this?

Is this a bad idea? Is there a better way to achieve the same effect?

// assume that "name" is a string passed as a parameter to this code block
try
{
    MainsDataContext dx = new MainsDataContext();
    try
    {
        Main m = dx.Main.Single(s => s.Name == name);
        return m.ID;
    }
    catch (InvalidOperationException)
    {
        Guid g = Guid.NewGuid();

        Main s = new Main 
        {
            Name = name,
            ID = g
        };

        dx.Mains.InsertOnSubmit(s);
        dx.SubmitChanges();

        return g;
    }
}
catch (Exception ex)
{
    // handle this
}

The objective here is to get the ID of a record if it exists, otherwise create that record and return it's ID.

Upvotes: 2

Views: 1630

Answers (5)

Shay Erlichmen
Shay Erlichmen

Reputation: 31928

No, but you might want to refector the inner block into an external method.

Upvotes: 0

Stan R.
Stan R.

Reputation: 16065

You should use SingleOrDefault, that way if a record doesn't exist it will return the default value for the class which is null.

MainsDataContext dx = null;    
try
    {
         dx = new MainsDataContext();

        Main m = dx.Main.SingleOrDefault(s => s.Name == name);

        if ( m == null)
        { 
           Guid g = Guid.NewGuid();

           m = new Main 
          {
              Name = name,
              ID = g
          };

         dx.Mains.InsertOnSubmit(m);
         dx.SubmitChanges();

        }

        return m.ID;
    }
    catch (Exception ex)
    {
        // handle this
    }
    finally
    {
       if(dx != null)
          dx.Dispose();
    }

it is a good idea to use the using keyword when using a DataContext

using ( MainsDataContext dx = new MainsDataContext())
{
        Main m = dx.Main.SingleOrDefault(s => s.Name == name);

        if ( m == null)
        { 
           Guid g = Guid.NewGuid();

           m = new Main 
          {
              Name = name,
              ID = g
          };

         dx.Mains.InsertOnSubmit(m);
         dx.SubmitChanges();

        }

        return m.ID;
}

Upvotes: 6

Pedro Sobota
Pedro Sobota

Reputation: 1877

Anyways, I think nested Try Catch blocks are good, like sibling Catch blocks each catching a different problem. It's good to be specific about errors. The catch-all should be only a safety net for production.

Upvotes: 0

TrueWill
TrueWill

Reputation: 25543

My question would be what code you intend to put in here:

// handle this

With the first catch block, you know that Single() threw an InvalidOperationException because the sequence contains more than one element or is empty.

In the second, you could get all kinds of errors. Null reference, data access, etc. How are you going to handle these?

Only catch what you know how to handle, and be as specific in the exception type as you can.

Upvotes: 1

Tamas Czinege
Tamas Czinege

Reputation: 121394

Main m = dx.Main.SingleOrDefault(s => s.Name == name);

if (m == default(Main))
{
    // it does not exist
}
else
{
    // it does exist
}

It is not apparent from the question if the type Main is a class or a struct (EDIT: I just realized that actually it must be a class), hence I used default() instead of just comparing to null.

Upvotes: 6

Related Questions