12

I am currently in a try catch finding if a property has been set properly to the bool value that it should be like this...

public void RunBusinessRule(MyCustomType customType)
{
    try
    {
       if (customType.CustomBoolProperty == true)
       {
            DoSomething(); 
       }
       else
       {
            throw new Exception("This is obviously false or possibly null lets throw up an error.");
       }
    }
    catch(Exception)
    {
        throw;
    }
}

Now the deal with throwing this error for me is that I am using Microsoft's source analysis and it gives me an error stating "CA2201 : Microsoft.Usage : Object.RunBusinessRule(MyCustomType)creates an exception of type 'Exception', an exception type that is not sufficiently specific and should never be raised by user code. If this exception instance might be thrown, use a different exception type.

Soooo What exception should I throw that would be specific enough for Microsoft.., for the circumstance of throwing an error about my own application's logic handling and when I want to "throw".

13 Answers 13

18
ArgumentException
InvalidOperationException
FormatException

The passed in argument wasn't good.

1
  • InvalidOperationException is "The exception that is thrown when a method call is invalid for the object's current state.", i.e. the class fields, not parameters.
    – brianary
    Apr 30, 2013 at 23:08
13

Create your own exception extending Exception. E.g.: RuleViolationException

13

Should you be throwing an exception at all?

Having a false boolean value isn't exactly an exceptional circumstance.

EDIT

My original answer was a bit terse so I'll elaborate...

From your example it's not clear what the actual objects, properties and methods represent. Without this information, it's difficult to say what type of exception, if any, is appropriate.

eg, I'd consider the following a perfectly valid use of an exception (and your real code might well look something like this, but we can't tell from your example):

public void UpdateMyCustomType(MyCustomType customType)
{
    if (!customType.IsUpdateable)
        throw new InvalidOperationException("Object is not updateable.");

    // customType is updateable, so let's update it
}

But in the general case, without knowing more about your domain model, I'd say that something like this (a false boolean value) isn't really exceptional.

4

ArgumentException maybe?

A case could be made for InvalidOperationException, too.

2

The answer here is that you shouldn't throw any exception. Why throw an exception just to catch it again in a second and rethrow it?

1

A slight aside, but you could simplify your code somewhat...

public void RunBusinessRule(MyCustomType customType)
{
    if (customType.CustomBoolProperty == false)
    {
        throw new Exception("This is obviously false or possibly null lets throw up an error.");
    }

    DoSomething(); 
}

As for the type of exception to throw, you might consider ApplicationException or InvalidOperationException, or you could define your own exception type.

1
  • Personaly, simplification like this always bothers me. As far as readability goes, I think if you are saying "Only do this if bool is true" then it should be part of the if/then statement. It is just my personal opinion, but I would always rather see explicit than implied. Feb 6, 2009 at 18:31
1

I know that a question is about throwing an exception but I think it would be more appropriate to do an assertation here:

// Precondition: customType.CustomBoolProperty == true
System.Diagnostics.Debug.Assert(customType.CustomBoolProperty)
DoSomething();
1

InvalidArgument exception is fine but better yet, an ApplicationException.

3
  • 3
    Microsoft used to recommend ApplicationException, but not any more - see msdn.microsoft.com/en-us/library/seyhszts.aspx for more info.
    – LukeH
    Feb 6, 2009 at 18:12
  • 1
    Actually, that post says not to create your own exceptions derived from ApplicationException; it doesn't say not to throw ApplicationException. Mar 23, 2012 at 20:23
  • 5
    Edit HOWEVER, Visual Studio code analysis says ApplicationException is "not sufficiently specific and should never be raised by user code" <sigh>. Mar 23, 2012 at 20:29
1

The other answers are fine for quick resolution, but ideally if you know at compile time that a certain method should never be invoked using certain parameters, you can prevent that from ever happening by inheriting your custom type, instanciating it only when that custom bool is true, and now your method looks like.

public void RunBusinessRule(MyInheritedType inheritedObject)
{
    //No need for checks, this is always the right type.
    //As a matter of fact, RunBusinessRule might even belong to MyInheritedType.
}

This is the I in SOLID.

0
1

I think you should avoid exceptions for code logic.

I suggest to modify your method to return the result of your method as a bool type then you may decide the appropriate way to show an error message to the user at the time of calling the method:

public bool RunBusinessRule(MyCustomType customType)
{
  try
    {
       if (customType.CustomBoolProperty == true)
       {
            DoSomething(); 
            return true;
       }

       return false;
    }
    catch(Exception)
    {
        throw;
    }
}
0

Make your own custom exception by extending System.Exception and throw that. You can get even crazier and have a whole tree of exception types if you want.

0

You could just create a custom ValidationException that is only used for your business logic validation. Or you could create a separate validation exception for each type of validation error although that is probably overload.

0

Not really what you were asking for, but there are plenty of people who have already given answers that I agree with, but you should also avoid only using catch(Exception ex).

It is a much better practice to try to catch the specific Exceptions that are possible first and if need be, catch the generic Expception. eg:

try{
   MyMethod(obj);
}catch (NullReferenceException ne){
   //do something
}
catch(UnauthorizedAccessException uae){
   //do something else
}
catch(System.IO.IOException ioe){
   //do something else
}
catch(Exception){
   //do something else
}
1
  • He will get the CA2201 with this code too as NullReferenceException and Exception at leat are not supposed to be used by something else than the runtime. Sep 20, 2012 at 12:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.