views:

209

answers:

4

I have the following try-catch statement and I do not want to not throw the exception if the message property contains 'My error' in the text.

How can I programmatcially accomplish this? Also, would this be considered code-smell?

try
{
}
catch(Exception e)
{
    if(e.Messages.Contains("My error"))
    {
       //want to display a friendly message and suppress the exception
    }
    else
    {
        throw e;
    }
}
+14  A: 

You shouldn't catch errors based on the error test. You should make your own exception class that extends exception:

class MyErrorException : Exception { }

and throw and catch those. (Excuse my syntax if it's wrong, I haven't done C# in a while).

That being said, throwing and catching your own Exceptions instead of propagating them is perfectly normal, and it is how you actually should do exception handling.

Claudiu
+1 - but that's java notation, in C# it would be: class MyErrorException : System.Exception { }
IanR
Claudiu is right, never look at the message of the exceptio nto see if this is right. By the way Claudiu, the right syntax is a colon to derive a class: `class MyException : Exception { }`.`
Marcel Gosselin
You (and all the other answerers so far) are assuming he controls the code which is throwing the exception. Is that definitely the case?
itowlson
@itowlson: Would throwing a general Exception for every type of error with a different message be a good reason to ditch a foreign library? Maybe not the main reason but at least a con against using it.
Yuriy Faktorovich
@itowlson: The code in the question does just that. We're just pointing out better ways.
Claudiu
+7  A: 

You should be catching the specific exception you're looking for. Quite frankly, that code is shocking. You should have something like ...

public class MyCoolException : Exception {
    public MyCoolException(string msg) : base(msg) {}
}

public void MyCoolMethod() {
    // if bad things happen
    throw new MyCoolException("You did something wrong!");
}

Then later in your code you can use it like ...

try {
    MyCoolMethod();
} catch (MyCoolException e) {
    // do some stuff
}
jcm
+1 for shocking. Although, it's really not, I've seen it way too many times to be shocked by it anymore :)
womp
+7  A: 

Your code creates maintainability issues because a simple text change can have strange side effects. You can have your own exception class which inherits from System.Exception. Then instead of having an if you could do the following:

try
{

}
catch(MyException myException) //or just catch(MyException)
{
    //display a friendly message
}

also you don't want to do throw e because it doesn't preserver the Stack, just throw; will do.

Yuriy Faktorovich
+1 for calling out the `throw e` statement as well. The correct way to rethrow an exception is simply `throw;`.
Aaronaught
Can I get a reason for the downvote?
Yuriy Faktorovich
A: 

When I throw Exception rather than a derived class I always mean a failed assertion. I don't like failing out the backend because we are still able to receive a request (just not that one again). If we're really toast it will just error out on the next request anyway.

When the back end needs to generate an error message I have a ErrorMessage class that inherits from Exception and takes ErrorMessage and ErrorMessageTitle as constructor arguments.

Joshua