0

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; } 
3
  • 4
    Worst kind of error handling ever. Commented Jan 26, 2016 at 10:12
  • 2
    Why are you using an empty catch block? Commented Jan 26, 2016 at 10:12
  • 1
    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? Commented Jan 26, 2016 at 10:15

3 Answers 3

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.

Sign up to request clarification or add additional context in comments.

4 Comments

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
@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.
This might be interesting, even if it is Java. stackoverflow.com/q/18679090/296526
Yeah I realized it! Your answer is perfect in this case!
1

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.

1 Comment

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
0

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...

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.