Reputation: 10247
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
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
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
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