We have this code which, when simplified, looks like this:
public class Room
{
public Client Client { get; set; }
public long ClientId
{
get
{
return Client == null ? 0 : Client.Id;
}
}
}
public class Client
{
public long Id { get; set; }
}
Now we have three viewpoints.
1) This is good code because the Client
property should always be set (i.e. not null) so the Client == null
will never occur and the Id value 0
denotes a false Id anyway (this is the opinion of the writer of the code ;-))
2) You can not rely on the caller to know that 0
is a false value for Id
and when the Client
property should always be set you should throw an exception
in the get
when the Client
property happens to be null
3) When the Client
property should always be set you just return Client.Id
and let the code throw a NullRef
exception when the Client
property happens to be null.
Which of these is most correct? Or is there a fourth possibility?
Best Answer
It smells like you should limit the number of states your
Room
class can be in.The very fact that you're asking about what to do when
Client
is null is a hint thatRoom
's state space is too large.To keep things simple I wouldn't allow the
Client
property of anyRoom
instance to ever be null. That means the code withinRoom
can safely assume theClient
is never null.If for some reason in the future
Client
becomesnull
resist the urge to support that state. Doing so will increase your maintenance complexity.Instead, allow the code to fail and fail fast. After all, this is not a supported state. If the application gets itself into this state you've already crossed a line of no return. The only reasonable thing to do then is to close the application.
This might happen (quite naturally) as the result of an unhandled null reference exception.