C# Code Quality – How to Implement a Property on Class A Which Refers to a Property of a Child Object of Class A

ccode-qualitynull

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 that Room's state space is too large.

To keep things simple I wouldn't allow the Client property of any Room instance to ever be null. That means the code within Room can safely assume the Client is never null.

If for some reason in the future Client becomes null 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.

Related Topic