Check property for null on every use?

Question

I just have a quick question to figure out what will be the best practice when creating my own class.

Let's say this class has one private member that is initialized in the constructor, should I then check if this private member is null in another public, non-stationary method? Or can it be saved so that the variable is not null and therefore it is not necessary to add this check?

For example, like the following: zero-need verification.

// Provides Client connections. public TcpClient tcpSocket; /// <summary> /// Creates a telnet connection to the host and port provided. /// </summary> /// <param name="Hostname">The host to connect to. Generally, Localhost to connect to the Network API on the server itself.</param> /// <param name="Port">Generally 23, for Telnet Connections.</param> public TelnetConnection(string Hostname, int Port) { tcpSocket = new TcpClient(Hostname, Port); } /// <summary> /// Closes the socket and disposes of the TcpClient. /// </summary> public void CloseSocket() { if (tcpSocket != null) { tcpSocket.Close(); } } 

So, I made some changes based on all of your answers, and I wonder if this might work better:

 private readonly TcpClient tcpSocket; public TcpClient TcpSocket { get { return tcpSocket; } } int TimeOutMs = 100; /// <summary> /// Creates a telnet connection to the host and port provided. /// </summary> /// <param name="Hostname">The host to connect to. Generally, Localhost to connect to the Network API on the server itself.</param> /// <param name="Port">TODO Generally 23, for Telnet Connections.</param> public TelnetConnection(string Hostname, int Port) { tcpSocket = new TcpClient(Hostname, Port); } /// <summary> /// Closes the socket and disposes of the TcpClient. /// </summary> public void CloseSocket() { if (tcpSocket != null) { tcpSocket.Close(); } } 

Thanks.

Answer 1

You have made the property public, so any code that uses this class can set a reference to null, as a result of which any operation on it will throw a NullReferenceException.

If you want the user of your class to live with it (which is protected): no, you do not need to check for null.

You can also make a property like public TcpClient tcpSocket { get; private set; } public TcpClient tcpSocket { get; private set; } public TcpClient tcpSocket { get; private set; } , so external code cannot set it to null. Unless you set tcpSocket to null inside your class, it will never be null, as the constructor will always be called.

Answer 2

I do not understand why you open the connection in ctor and then the public method to close it. If you create a connection in ctor, this usually means that this is the connection you want for the class to live.

If you are asking how to make sure the connection is closed when the class is located, then implement IDisposable.

IDisposable Interface

Since it is closed, it should not be null, but you should check for it.

  if (tcpSocket.Connected) { tcpSocket.Close(); } 

Answer 3

It is generally safe if you can make sure that the field is not zero. You can call it class invariant. However, in your code, tcpSocket not private, so anyone can set it to null .

I would recommend that you create a field with a private setter (unless you make it completely private). This makes you sure that no external (i.e. uncontrolled!) Code changes the link. This in turn allows you to guarantee tcpSocket not null .