7

Assume I have a class like this:

public class Foo
{
    public Bar RequiredProperty { get; set;}

    public void Baz()
    {
        if (this.RequiredProperty == null)
        {
            // Which exception should I throw?
        }
    }
}

My solution has a class that is designed to be reused without having to pass a lot of arguments to the Bar method over and over again. So, what should I throw when Bar isn't initialized to a non-null value?

More Information I'm essentially rolling my own code parser and formatter. Call it an object lesson. One of the classes is an HtmlCodeFormatter that has the following properties (in honor of Dependency Injection):

public IFormatter Formatter { get; set; }
public IParser Parsre { get; set; }

This allows me to write any number of language-specific parsers and formatters. For example, I have a CSharpParser and a JavascriptParser. I also have an HtmlCodeFormatter, and there are plans for another (of dubious utility).

The idea is that you can instantiate the HtmlFormatter using an object initializer, like this:

var formatter = new HtmlCodeFormatter()
    {
        Parser = new CSharpParser();
        Formatter = new HtmlCodeFormatter();
    };
formatter.Format("Console.WriteLine(\"Hello, world!\"));

When HtmlCodeFormatter.Format is invoked, it needs to be able to verify that both a parser and a formatter have been provided. That's no problem, really, but I'm kind of stumped about which exception to throw. I'm leaning towards InvalidOperationException, but I'm not entirely certain that's the best choice.

Mike Hofer
  • 16,477
  • 11
  • 74
  • 110

3 Answers3

15

I would throw a InvalidOperationException. The MSDN definition is:

The exception that is thrown when a method call is invalid for the object's current state.

However, i recommend using constructor injection instead of setter injection. This makes sure you has valid parsers and formatters. If null is passed in the constructor, throw a ArgumentNullException.

alexn
  • 57,867
  • 14
  • 111
  • 145
7

By using Propery Injection you are implicitly telling the compiler, fellow developers as well as DI Containers that the dependency is optional. This is because no-one forces you to assign a property value.

If the dependency is truly required, you should refactor to Constructor Injection. This will effectively make the dependency required:

public class Foo
{
    private readonly Bar bar;

    public Foo(Bar bar)
    {
        if (bar == null)
        {
            throw new ArgumentNullException("bar");
        }

        this.bar = bar;
    }

    public Bar RequiredProperty
    {
        get { return this.bar; }
    }

    public void Baz()
    {
        // this.bar is now GUARANTEED to be initialized,
        // so no checks are required
    }
}

Notice that the constructor Guard Clause (throwing an ArgumentNullException) effectively protects the invariants of the class, which is further strengthened by the readonly keyword.

With the dependency part of the class' invariants, you no long have any temporal coupling and you can now implement all members without worrying whether the dependency has been assigned or not.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • I can definitely see your point, but I was under the impression that it's A Very Bad Idea™ to throw exceptions in a constructor, and that it was to be avoided At All Costs™. – Mike Hofer Mar 05 '11 at 20:34
  • 1
    Not at all. The principle is called Fail Fast. Where did you get that idea? – Mark Seemann Mar 05 '11 at 20:47
  • You know, I don't really know. There *is* the nagging chance that I'll get a TypeInitializationFailed exception, which might mask the real problem. But I *am* a fan of fail fast, so I've fixed my code with your guidance in mind. – Mike Hofer Mar 05 '11 at 20:52
1

You could throw NotInitializedException.

decyclone
  • 30,394
  • 6
  • 63
  • 80