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

It is 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 goal is to get the id of the entry if it exists, otherwise create this entry and return its id.

+2


source to share


5 answers


You have to use SingleOrDefault so if the entry doesn't exist it will return the default 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 recommended to use the c keyword when using 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;
}

      

+6


source


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

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

      



It's not clear if the type is Main

a class or a struct
(EDIT: I just realized that it should actually be a class), so I used default()

instead of comparing with null

.

+6


source


My question will be about what code do you intend to enter 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 case, you can get all sorts of errors. Null reference, data access, etc. How do you handle this?

Just understand that you know how to handle it, and be as specific as possible about the type of exceptions.

+1


source


No, but you might want to convert the inner block to an outer method.

0


source


Anyway, I think nested Try Catch blocks are good, for example sibling Catch blocks each one catching a different problem. It would be nice to talk about mistakes. The trick is to only be a safety net for production.

0


source







All Articles