Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Check property for null upon every use?

I just have a quick question to see what the best practice would be when making your own class.

Let's say this class has one private member which gets initialized in the constructor, would I have to then check to see if this private member is null in another public, non-static method? Or is it save to assume that the variable will not be null and therefore not have to add that check?

For example, like the following, is the check for null absolutely necessary.

// 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 have made some changes based on all your answers, and I'm wondering if maybe this will 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.

like image 917
Feytality Avatar asked Jan 15 '23 18:01

Feytality


1 Answers

You've made the property public, so any code using this class can set the reference to null, causing any operation on it to throw a NullReferenceException.

If you want the user of your class to live with that (which is defendable): no, you don't have to check for null.

You could also make the property like public TcpClient tcpSocket { get; private set; }, so external code can't set it to null. If you don't set the tcpSocket to null inside your class, it will never be null since the constructor will always be called.

like image 131
CodeCaster Avatar answered Jan 25 '23 03:01

CodeCaster