Reputation: 7424
So I am recently writing a relatively complex application written in C# that performs an array of small tasks repeatedly. When I first started the application I realized that a lot of the code I was typing was repetitive and so I began encapsulating the majority of the app's logic into separate helper classes that I could call as needed.
Needless to say the size of my app (and amount of code) was cut in half. But as I was going through I noticed something else in my application that seemed to be repetitive and looked like it could be improved.
Now most of my methods in my helper classes are either making a HttpWebRequest
or performing save/delete operations on files. Having said that I need to handle the possibility that eventually the call won't complete, or the file can't save because there isn't enough space, or whatever. The problem I'm running into is that I have to keep writing try/catch statements every time I call one of the methods. On top of that I have to retype the error message (or Eventually a status message. I would like to know when it succeeds as well).
So here's kind of a snippet of what I have to type:
try
{
ItemManager.SaveTextPost(myPostItem);
}
// Majority of the time there is more than one catch!
catch
{
//^^^Not to mention that I have to handle multiple types of exceptions
//in order to log them correctly(more catches..ugh!)
MessageBox.Show("There was an error saving the post.");
//Perform logging here as well
}
From what I have concluded so far is:
Is this approach correct? Is there another way of doing this? Should I be using a try/catch in the calling method at all? Since this kind of my first shot at this I would really like to hear what others who have handled this scenario have to say.
Upvotes: 5
Views: 300
Reputation: 61
Try/Catch is a good solution. One suggestion, I like to use this to stem the flow of errors: exception catching: try { } catch (Exception ex) { ShowExceptionError(ex); } Then I will simply throw all exceptions to a single method, and let the method handle each.
Kind of like this:
private void ShowExceptionError(ex)
{
string errorMessage = "Type of error:" + ex.GetType().ToString();
string stackTrace = ex.StackTrace();
MessageBox.Show(errorMessage + "\n\n" + stackTrace);
}
Upvotes: 1
Reputation: 18743
You should throw exceptions in your helper methods by checking with if
blocks if every argument is correct, every file is reachable... then if needed, implement some try catch
blocks in your helper methods and throw exceptions when catching.
Finally, enclose these helper methods by a try catch
block, but at this time, really catch the exceptions:
try
{
ItemManager.SaveTextPost(myPostItem);
// SaveTextPost can throw some exceptions (that you know)
}
catch (Exception e)
{
// You know the exceptions that SaveTextPost can return, so threat them
// correctly
if (e is FileNotFoundException)
MessageBox.Show("The file was not found when saving the post.");
else if (e is ArgumentNullException)
MessageBox.Show("The post can't be null to be saved.");
else
MessageBox.Show("There was an error saving the post.");
}
At the end, you need to treat the error and show an error message to the user. You only can decide if the MessageBox.Show
should be implemented in the helper method or in the class calling the helper method.
Personally, I think helper methods are destined to be used by any developer that will run your code, so you should let him decide what he wants to do with the error. That means throwing exceptions in the helper method.
Upvotes: 1
Reputation: 51
All of these ways are great options. One thing you want not to do is to use a try {} catch {} for flow control. This means something like this (again avoid this)
Void SomeMethod()
{
try
{
while(somecondition)
{
try
{
}
catch (Exception ex)
{
}
}
}
catch (Exception ex)
{
.....
}
Instead you want to code defensively.
Upvotes: 1
Reputation: 6876
My personal take on exception handling is.. add try {} catch {}
where it makes sense.
For example always use a try catch when calling "untrusted" code, i.e. modules or plugins. otherwise if you are going to catch an exception make sure you can do something meaningful with it. If not, allow the exception to bubble up to a point where you can.
There are few things worse than trying to debug an app where the developer caught an exception and returns a bool. Just sayin'
Upvotes: 0
Reputation: 15702
There is no single perfect rule for exception handling, so the answer is: It depends. In general you should only catch exceptions, if you know how to handle them. In your example my first question would be, if the application can continue to run after this error and would still be in a valid state. If not, you should rethrow the exception after logging. Then think about nesting exception: You can catch an exception, add information by nesting it into another and throwing that exception. If you design your exception classes and handlers carefully, your logging and displaying code should become much simpler than you expect it. But the details obviously depend on your application.
Upvotes: 1
Reputation: 611
I think putting the try/catch in the method you are calling is perfectly fine. You can return error/success codes to the calling code in a variety of ways. .NET and c# handle enumerations nicely, so you could have ItemManager.SaveTextPost(myPostItem, out errorCode);
where errorCode would be an enumerated value that would let you know if any problems occurred. It could also be as simple as having the method return a bool true if successful, false if otherwise. There are many ways that you could handle that issue, but as far as I'm concerned putting the try/catch in the method is the preferable way of doing things.
Upvotes: 1
Reputation: 83358
AOP libraries like PostSharp are designed to handle cross-cutting concerns exactly like this.
If all these helper methods employ the same boilerplate try catch -> handle multiple types of exceptions, then you can write one aspect that does that, and decorate all of the relevant methods with the appropriate attribute.
Upvotes: 1