47

In my application I need to throw an exception if a property of a specific class is null or empty (in case it's a string). I'm not sure what is the best exception to use in this case. I would hate to create a new exception and I'm not sure if ArgumentNullException is appropriate in this case.

Should I create a new exception or there's an exception I can use?

I don't mind to throw an ApplicationException.

Vadim
  • 21,044
  • 18
  • 65
  • 101
  • Take in mind too properties are syntactic sugared methods. – Dykam Sep 28 '09 at 19:00
  • A related interesting discussion: http://stackoverflow.com/questions/1488472/best-practices-throwing-exceptions-from-properties – Joren Sep 28 '09 at 19:08

9 Answers9

73

The MSDN guidelines for standard exceptions states:

Do use value for the name of the implicit value parameter of property setters.

The following code example shows a property that throws an exception if the caller passes a null argument.

public IPAddress Address
{
    get
    {
        return address;
    }
    set
    {
        if(value == null)
        {
            throw new ArgumentNullException("value");
        }
        address = value;
    }
}

Additionally, the MSDN guidelines for property design say:

Avoid throwing exceptions from property getters.

Property getters should be simple operations without any preconditions. If a getter might throw an exception, consider redesigning the property to be a method. This recommendation does not apply to indexers. Indexers can throw exceptions because of invalid arguments.

It is valid and acceptable to throw exceptions from a property setter.

So throw ArgumentNullException in the setter on null, and ArgumentException on the empty string, and do nothing in the getter. Since the setter throws and only you have access to the backing field, it's easy to make sure it won't contain an invalid value. Having the getter throw is then pointless. This might however be a good spot to use Debug.Assert.

If you really can't provide an appropriate default, then I suppose you have three options:

  1. Just return whatever is in the property and document this behaviour as part of the usage contract. Let the caller deal with it. You might also demand a valid value in the constructor. This might be completely inappropriate for your application though.

  2. Replace the property by methods: A setter method that throws when passed an invalid value, and a getter method that throws InvalidOperationException when the property was never assigned a valid value.

  3. Throw InvalidOperationException from the getter, as you could consider 'property has never been assigned' an invalid state. While you shouldn't normally throw from getters, I suppose this might be a good reason to make an exception.

If you choose options 2 or 3, you should also include a TryGet- method that returns a bool which indicates if the property has been set to a valid value, and if so returns that value in an out parameter. Otherwise you force callers to be prepared to handle an InvalidOperationException, unless they have previously set the property themselves and thus know it won't throw. Compare int.Parse versus int.TryParse.

I'd suggest using option 2 with the TryGet method. It doesn't violate any guidelines and imposes minimal requirements on the calling code.


About the other suggestions
ApplicationException is way too general. ArgumentException is a bit too general for null, but fine otherwise. MSDN docs again:

Do throw the most specific (the most derived) exception that is appropriate. For example, if a method receives a null (Nothing in Visual Basic) argument, it should throw System.ArgumentNullException instead of its base type System.ArgumentException.

In fact you shouldn't use ApplicationException at all (docs):

Do derive custom exceptions from the T:System.Exception class rather than the T:System.ApplicationException class.

It was originally thought that custom exceptions should derive from the ApplicationException class; however, this has not been found to add significant value. For more information, see Best Practices for Handling Exceptions.

InvalidOperationException is intended not for when the arguments to a method or property are invalid, but for when the operation as a whole is invalid (docs). It should not be thrown from the setter:

Do throw a System.InvalidOperationException exception if in an inappropriate state. System.InvalidOperationException should be thrown if a property set or a method call is not appropriate given the object's current state. For example, writing to a System.IO.FileStream that has been opened for reading should throw a System.InvalidOperationException exception.

Incidentally, InvalidOperationException is for when the operation is invalid for the object's current state. If the operation is always invalid for the entire class, you should use NotSupportedException.

Joren
  • 14,472
  • 3
  • 50
  • 54
  • 1
    @Joren, @Vadim indicates that he needs a default instructor and doesn't have a reasonable default for this field, so just monitoring the setter won't be enough for his scenario. – bdukes Sep 28 '09 at 18:19
  • I agree with all that's said here. I just want to add about `ApplicationException` that's a nice one to use if you use it for 1 sweeping purpose like any business software state exceptions. It's not that the system *can't* do that, it's that it won't. It would be terrible if you threw `ApplicationException` for everything, but i'd rather just use AE than inherit from it. Inheriting to just rename something is wasted effort. – Chris Marisic Apr 08 '14 at 13:04
7

I would throw an InvalidOperationException. MSDN says it "is thrown when a method call is invalid for the object's current state."

bdukes
  • 152,002
  • 23
  • 148
  • 175
  • 1
    I don't think that's appropriate for this scenario. InvalidOperationException should only be used when the object being called is invalid. In this case the object being called is 100% valid, it's the argument to the method that is invalid. – JaredPar Sep 28 '09 at 17:52
  • 2
    You would throw `InvalidOperationException` if the object were in such a state that it would be invalid to use the property setter at all. That is, the *operation* itself is invalid. The MSDN docs give the example of "writing to a `System.IO.FileStream` that has been opened for reading". – Joren Sep 28 '09 at 17:54
  • I suppose we don't know the exact scenario of what's being called. I am working under the assumption that we're calling a method of a class, and trying to use a property of that class within the method. In that case, the object is invalid for the method that is being called (since the property needs to be set before the method can be called). It looks like @JaredPar is considering the scenario where a property on an argument is not initialized. In that case, his suggestion of `ArgumentException` makes the most sense. – bdukes Sep 28 '09 at 18:15
  • I should add that my previous comment was purely considering the property setter. The property not being assigned to a valid value is indeed an example of invalid state. I don't think you should throw that exception from a property getter though ... there are other options. – Joren Sep 28 '09 at 18:55
  • 1
    If the state of the encapsulating object becomes invalid/non-functional because the property you are setting is invalid then I think InvalidOperationException is completely acceptable. However in such a scenario I guess that the property should be injected via the constructor as the encapsulating object depends on that property in order to fulfill it's single responsibility. – ComeIn Mar 09 '15 at 02:37
  • I second the answer! I have an object that has a search-method and there is a property that says search _binary_ or by _brute force_. Binary is not supported yet (because buggy). So I want to throw an exception when the property is set to _binary_ when the search-method is called. Vadim didn't say anything about setters. – Bitterblue Sep 05 '16 at 08:11
3

If the problem is that a member of an argument, and not the argument itself, is null then I think the best choice is the more generic ArgumentException. ArgumentNullException does not work here because the argument is in fact not null. Instead you need the more generic "something is wrong with your argument" exception type.

A detailed message for the constructor would be very appropriate here

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • What do you mean "a detail message for the constructor"? I must have a default constructor. – Vadim Sep 28 '09 at 17:43
  • @Vadim a detailed reason about why the argument exception is being thrown. For example "Property Blah of parameter foo must be non-null" – JaredPar Sep 28 '09 at 17:50
  • @Vadim, he's referring to the exception's constuctor, not your object's constructor. – bdukes Sep 28 '09 at 18:16
2

Well, it's not an argument, if you're referencing a property of a class. So, you shouldn't use ArgumentException or ArgumentNullException.

NullReferenceException would happen if you just leave things alone, so I assume that's not what you're looking for.

So, using ApplicationExeption or InvalidOperationException would probably be your best bet, making sure to give a meaningful string to describe the error.

John Fisher
  • 22,355
  • 2
  • 39
  • 64
  • 1
    I'm pretty sure there are classes in the .NET Framework that throw ArgumentException from a property's setter. I can't remember where exactly but I do remember coming across it. You could argue that it might not be "proper" but regardless the framework has set the precedent. – Tinister Sep 28 '09 at 17:40
2

It is quite appropriate to throw ArgumentNullException if anyone tries to assign null.

A property should never throw on a read operation.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • Where do you get the idea that a property should never throw on a read operation? It certainly can happen easily enough. – John Fisher Sep 28 '09 at 17:37
  • 1
    @John I don't think his point was that it can't but merely that it shouldn't. – Brian Rasmussen Sep 28 '09 at 17:41
  • 1
    @John Fisher: Cwalina, Krzystof and Brad Abrams: "Framework Design Guidelines. Conventions, Idioms, and Patterns for Reusable .Net Libraries", Addison-Wesley, 2006. p. 121 - among other sources. – Mark Seemann Sep 28 '09 at 17:48
  • Ah. I certainly understand "avoid throwing exceptions from property getters", but that is quite different from "should never ...". – John Fisher Sep 28 '09 at 18:28
2

If it can't be null or empty, have your setter not allow null or empty values, or throw an ArgumentException if that is the case.

Also, require that the property be set in the constructor.

This way you force a valid value, rather than coming back later and saying that that you can't determine account balance as the account isn't set.

But, I would agree with bduke's response.

James Black
  • 41,583
  • 10
  • 86
  • 166
1

Does the constructor set it to a non-null value? If so I would just throw ArgumentNullException from the setter.

Tinister
  • 11,097
  • 6
  • 35
  • 36
1

There is a precedent for stretching the interpretation of ArgumentNullException to meaning "string argument is null or empty": System.Windows.Clipboard.SetText will throw an ArgumentNullException in this case.

So I wouldn't see anything wrong with using this rather than the more general ArgumentException in your property setter, provided you document it.

Joe
  • 122,218
  • 32
  • 205
  • 338
0

Just throw whatever as long as the error message is helpful to a developer. This class of exception should never happen outside of development, anyway.

erikkallen
  • 33,800
  • 13
  • 85
  • 120