try/catch and returning values

Question

I have a method that returns a List. Now I want to know how to place the try/catch blocks properly. If I place the return statement inside try I get error

Not all code paths return a value

If I place after catch(like I'm doing currently) it will return the products even after an Exception. So what should be the best way?

Here is the method:

public List<Product> GetProductDetails(int productKey)
{
    List<Product> products = new List<Product>();
    try
    {
       using (SqlConnection con = new SqlConnection(_connectionString))
       {
         SqlCommand cmd = new SqlCommand("usp_Get_ProductDescription", con);
         cmd.CommandType = CommandType.StoredProcedure;
         cmd.Parameters.AddWithValue("@riProductID", productKey);
         con.Open();
         using (SqlDataReader reader = cmd.ExecuteReader())
         {
           while (reader.Read())
           {
             Product product = new Product(reader["Name"].ToString(), reader["Code"].ToString());
             products.Add(product);
           }
         }
       }
     }
     catch { }
     return products;
}

In the event of an exception what do you want to happen? Do you want it to return all the products it has found so far? Skip any products that cause an error? Return an empty list? Return null? — Ian Johnson, Jan 26 '16 at 10:15

Myrtle · Accepted Answer · 2016-01-26T10:20:12.183

4

Remove the complete Try and Catch blocks. Apparently you are unable to handle the exceptions in the GetProductDetails method so just let them be thrown.

However the calling code can make the decision:

IList<Product> products = null;

try
{
    products = GetProductDetails(3);
}
catch(Exception ex)
{
    // Here you can make the decision whether you accept an empty list in case of retrieval errors.
    // It is the concern of this method, not of the ProductDetails method.
    // TODO: Use logging
    products = new List<Product>();
}

I can imagine it feels like code duplication if you have to write this in every method using the GetProductDetails method. However consider, when you have X implementations, you want to react differently to not being able to get product details. You will have to come up with workarounds. You might even end up with weird bugs which are hard to troubleshoot.

edited Jan 26 '16 at 10:20

answered Jan 26 '16 at 10:13

Myrtle

5,761
33
47

Could you plz suggest any good resource where I can get an idea of error handling and what exceptions one must always catch? I'm asking because a lot of people suggested to handle exception after this post – Huma Ali Jan 27 '16 at 08:48
1

@HumaAli those lots of people tend to directly solve your problem. They are correct in the case you want to handle the exception in your method. However,I proposed a better design which removes your initial problem at all. – Myrtle Jan 27 '16 at 11:40
This might be interesting, even if it is Java. http://stackoverflow.com/q/18679090/296526 – Myrtle Jan 27 '16 at 11:41
Yeah I realized it! Your answer is perfect in this case! – Huma Ali Jan 27 '16 at 11:55

score 1 · Answer 2 · answered Jan 26 '16 at 10:29

That depends on what should happen in an exceptional case. If this might happen for some reason which isn´t "bad enough" to let the app crash or if you are able to handle that exception appropriately then you might go with your current appraoch - however you should definitly leave at least a log-message within the catch-clause that contains the error which has been thrown:

catch (Exception e) 
{ 
    log.Info(e.Message);
}

This way you get all results within your list except those that caused any exceptions. You can simply continue work with all the results that you got and ignore those errorous (supposing you logged them in any way).

If it is a really unexpected behaviour (which is the intended one for exceptions, this is why they are called exceptions) you should delete all this try/catch from within your method and handle any exceptions outside the method as Maurice already mentioned.

Could you plz suggest any good resource where I can get an idea of error handling and what exceptions one must always catch? I'm asking because a lot of people suggested to handle exception after this post — Huma Ali, Jan 27 '16 at 08:48

Jules · Answer 3 · 2016-01-26T10:32:51.193

At the moment, you don't return anything if an exception is thrown. Use try, catch, finally. (May you want to see the MSDN page for more official information)

try
{
    //try to execute this code
}
catch
{
    //execute this if an exception is thrown
}
finally
{
    //execute this code, after try/catch
}

So if you place your return statement into the finally section, you will return your list even if there's an exception thrown...

try/catch and returning values

3 Answers3