11

So I heard that validating a value in a property like this:

//dummy example, let's assume that I want my value without dots
public string MyProp
{
    set
    {
        if(value.Contains('.'))
            throw new ArgumentException("Must not contain '.'", "value");
    }
}

is wrong and I should avoid it.

But in earlier days I was told that this is the good way. We could use encapsulation, there is just one place to check, DRY, etc.

What's wrong with my little example?

Dave Clemmer
  • 3,741
  • 12
  • 49
  • 72
ramires.cabral
  • 910
  • 4
  • 13
  • 28

3 Answers3

13

There is nothing wrong with throwing exceptions in a property setter. But you should throw an ArgumentException, and also actually set the value of the property!

private string _myprop;
public string MyProp{
    set{
       if(value.Contains('.')) throw new ArgumentException("Must not contain .");
       this._myprop=value;
    }
    get { return this._myprop; }
}

From an article on best practices in MSDN:

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.

Community
  • 1
  • 1
Julián Urbano
  • 8,378
  • 1
  • 30
  • 52
0

There are some similar questions also on SO.

You properities should be a lightweight as possible. And if setters throws an error this is ok but again you might consider moving it to a function. Things can get messy easily.

AVOID throwing exceptions from property getters. Property getters should be simple operations and should not have preconditions. If a getter can throw an exception, it should probably be redesigned to be a method.

Best practices: throwing exceptions from properties

What exception to throw from a property setter?

Community
  • 1
  • 1
adt
  • 4,320
  • 5
  • 35
  • 54
-2

Please see: Best practices: throwing exceptions from properties for an explanation and discussion on why throwing exceptions from properties is bad.

Admittedly that post speaks about property getters.

Setters are generally seen by consumers as simply setting a private field hidden by the property and maybe doing some additional stuff as required, exceptions are unexpected behaviour and could you imagine having to enclose each set statement inside a try block?

While it may be accepted behaviour by guideline, it's a nightmare for guessing for developers, and validation logic should be contained in a separate method and then called from the property if need be.

If you need the validation or special behaviour when setting properties you should use a set method e.g. SetMyProp(string value) as it brings a distinction to it that it may lead to an exception etc.

If you're using the properties for something like a WPF model then you should make use of WPF's built-in validation for data instead.

Community
  • 1
  • 1
Clint
  • 6,133
  • 2
  • 27
  • 48
  • 1
    -1 the very article you link to states that exceptions in **setters** are ok. From MSDN: "It is valid and acceptable to throw exceptions from a property setter." – Julián Urbano Apr 08 '13 at 17:35
  • Do you see a problem with a setter doing validation (and throwing an exception) to enforce it's contract (for example not allowing a null value to be set)? – hatchet - done with SOverflow Apr 08 '13 at 17:37
  • @hatchet yes, I think for null values it's acceptable or for edge cases where the value *really* mustn't take on a certain form, but if it's a value that is "wrong" in the context of further use, the exceptions should take place when they're attempted to be used. – Clint Apr 08 '13 at 17:39
  • Why guess when you have docstrings? And your point of having a `try` block around every set statement is akin to using a `try` block around every `Substring` because your starting index could be out of bounds. Just check the data. – Ry- Apr 08 '13 at 17:53
  • I disagree. It's perfectly acceptible for setters to throw exceptions. How else will you handle for example a property which is not allowed to be null? If you remove the setter, serialization will fail. The ability to do `Contract.Requires(value != null);` is essential. – Matthew Watson Apr 08 '13 at 18:36