3

Suppose you have a private variable like so

private int _x;

And you have a property that provides external access to this variable:

public int X 
{
    get 
    {
        return _x;
    }
    set
    {
        _x = value;
    }
} 

Is it better to put "validation" logic (value non-negative, within bounds, etc) in the getter portion or the setter portion? It seems like it might be acceptable in either, but is there a preferred option?

  • 3
    Putting it in the getter means the caller will never know they're doing something wrong. – DavidG Dec 23 '16 at 16:24
  • 5
    You'd want it in the setter because there's no point in validating it every time you retrieve the value, unless you update _x directly for some reason. – itsme86 Dec 23 '16 at 16:24
  • 3
    Adding to what @DavidG said... WHat are you validating if you put in the getter ? Validation is generally (always?) validating user input, to make sure that corrupt (invalid) data does not get *into* the system. It goes in the setter. – Charles Bretana Dec 23 '16 at 16:27
  • 1
    Right, that makes sense. How do I mark this question as answered? I'm new here (to posting questions, not to viewing them). – Gaston Gilbert Dec 23 '16 at 16:27
  • 1
    have @David put his comment in an answer, and mark it as accepted. – Charles Bretana Dec 23 '16 at 16:28
  • Reasons to validate in a getter: (a) You have allowed a value for a property, e.g. `null`, at construction that should never see the light of day. (ii) You are using lazy initialization, e.g. doing an expensive network or database operation, only when a value is actually requested and, at that time, detect a problem. A setter should always* perform validation. (* - Okay. There's always the 1 in 10^6 realtime logging thing that doesn't care and can't wait for an expensive check. So it goes.) – HABO Dec 23 '16 at 16:42

4 Answers4

3

The setter is preferred, for the following reason: It is better to throw and exception or display a message to the user upon inputting a garbage value, rather than allowing the garbage value, and subjecting your class to internal bad data.

You will note that the MSDN Example uses the setter for input validation.

vbnet3d
  • 1,151
  • 1
  • 19
  • 37
  • The example you referred to is probably not meant to promote this practice, it is probably shown to demonstrate the syntax only. In fact it does not even do anything when there is an error. – NoChance Oct 16 '20 at 22:24
3

You want your code to fail as quickly as possible, which is at the point you try to set an invalid value.

When you fail in the setter, the user knows about the problem immediately, and can fix it. If you wait until they try to retrieve the value, you are waiting too late, and the user may have no idea what went wrong, or where.

If the invalid value gets used elsewhere in the code, you are propagating bad data throughout the application, making things much worse, and even less clear to the user what went wrong.

2

The validation logic should be in the setter, to prevent invalid data from even reaching _x. That way, you have a useful invariant in your class: _x will always contain a valid value.

The idiomatic way to perform validation is to throw an ArgumentException or any of its subclasses when the consuming code tries to assign an invalid value to X.

Community
  • 1
  • 1
Heinzi
  • 167,459
  • 57
  • 363
  • 519
1

Validation should be called first. If you want to use this approach you should implement your logic in set clause. If you want create nice clean code, you should consider dedicated method for it, e.g.:

public class Test
{
    public int X { get; private set; }

    public void SetX(int value)
    {
         //... your logic, throw exception if validation failed
         X = value;
    }
}

Your class should keep your object in valid state.

Pawel Maga
  • 5,428
  • 3
  • 38
  • 62