Is the property accessor/mutator (getter/setter) the right place to enforce acceptable ranges?

In an app where a port number is set, I want to restrict the values assignable to the port number to those between 49152 and 65535 (inclusive).

I wrote some test methods that test that anything outside of this range causes the test to fail. They do fail (as expected, as the code does not yet take that into account).

So my question is: what is the best place to put the code that forces an invalid value to a valid value - here:

public int Port
{
    get
    {
        return port;
    }
    set
    {
        port = value;
    }
}

such as:

public int Port
{
  get
  {
      return port;
   }
   set
   {
       if ((value < 49152) || (value > 65535))
       {
          value = 55555;
       }
       port = value;
    }
}

...or somewhere else?

Upvotes: 2

Views: 2507

Answers (3)

qxn
qxn

Reputation: 17584

You can put validation logic in your setter, but you should probably let somebody know if the validation fails.

ie:

private static int minimumPortNumber = 49152;
private static int maximumPortNumber = 65535;

public int Port
{
  get
  {
      return port;
   }
   set
   {
       if ((value < minimumPortNumber) || (value > maximumPortNumber))
       {
           throw new ArgumentOutOfRangeException("port", string.Format("The port number is out of range. It must be between {0} and {1}", minimumPortNumber, maximumPortNumber));
       }
       port = value;
    }
}

Upvotes: 12

Eric Lippert
Eric Lippert

Reputation: 660088

what is the best place to put the code that forces an invalid value to a valid value?

The best place for that is nowhere. Forcing an invalid value to a random valid value is hiding a bug in the caller. The caller can know ahead of time whether the value they are attempting to set is valid or not, so crash them if they do it wrong. Throw an exception, don't ignore the bug and make a guess as to what they meant.

Upvotes: 15

Brandon Moretz
Brandon Moretz

Reputation: 7621

This is one of the biggest benefits to having wrapper methods around the fields and not exposing them directly. So I would say most definitely, yes.

Upvotes: 2

Related Questions