Unnecessary NullReferenceException?

Question

Why do I get a NullReferenceException in the following code?

private string FileR ( string name )
{
    string[] content = ReadSite(name, Protocol.read, url);
    Request_Template newCon;
    string[] final = new string[content.Length];

    for (int i = 0; i < content.Length; i++)
    {
        if (content[i].Equals(null))
        {
            return "content [" + i + "] returns null";
        }
        newCon = JsonSerializer.Deserialize<Request_Template>(content[i]);

        if (newCon.Sender.Contains(myAccount.Username))
        {
            newCon.Sender = "Me";
        }

        string sender = newCon.Sender;
        string message = newCon.Message;
        final[i] = sender + ":\t" + message;
    }

    string nFinal = string.Concat(final);

    Thread.Sleep(10);
    return nFinal;
}

string[] ReadSite(string filename, Protocol p, string uri)
{
    Read_Template temp = new Read_Template
    {
        Chat = filename,
        Code = key1
    };
    string myUrl = JsonSerializer.Serialize(temp);

    WebClient web = new WebClient();
    Stream stream = web.OpenRead(uri + "/" + myUrl);

    int length = 0;
    while (new StreamReader(stream).ReadLine() != null)
    {
        length++;
    }
    string[] content = new string[length];

    using (StreamReader reader = new StreamReader(stream))
    {
        for (int i = 0; i < length; i++)
        {
            content[i] = reader.ReadLine();
        }
    }

    return content;
}

I've tried using the debugging tools with no avail. When I ran the code, it said that the error came from string [] final = new string [content.Length]; and for (int i = 0; i < content.Length; i++).That lead me to assume that content was null. But when I used the watch window and it said that the variable content cannot be determined. How do I fix this?

Answer 1

I strongly suspect that the problem is actually here:

if (content[i].Equals(null))

If content[i] is genuinely null, then calling content[i].Equals will throw a NullReferenceException. That test should be written as:

if (content[i] is null)`

Or (for C# 6 or older):

if (content[i] == null)`

Now if ReadSite didn't have a bug in, you shouldn't need that check at all - because ReadSite should return an array of non-null string references. However, the way that you're populating the array is broken. You're currently:

• Creating a StreamReader around the stream
• Reading from the reader until you run out of data
• Creating a new array of the "right" length
• Creating another StreamReader around the same stream - which is by now at the end
• Reading from the reader however many times you read a line originally

Because the stream is already at the end, ReadLine() is going to return null on every iteration.

Instead, you should just read once, populating a list as you go, like this:

List<string> content = new List<string>();
using (var stream = web.OpenRead(uri + "/" + myUrl))
{
    using (var reader = new StreamReader(stream))
    {
        string line;
        while ((line = reader.ReadLine()) is object)
        {
            content.Add(line);
        }
    }
}
return content;

That's returning a List<string>, so you'd want to change your ReadSite return type to List<string> or perhaps IReadOnlyList<string>. If you really need it to return an array, you could return content.ToArray() instead.

(Ideally, move to using HttpClient as well, but that's a different story.)

Answer 2

private string FileR ( string name )
{
    // ReadSite always returns a content that has length 0 or more. string[] content will never equals null. 
    // TEST: string[] test = new string[0]; test == null -> False
    string[] content = ReadSite(name, Protocol.read, url);
    Request_Template newCon;

    string[] final = new string[content.Length];

    // if content.Length == 0, this loop will never occur and you wont get any NREs.
    for (int i = 0; i < content.Length; i++)
    {
        // It is highly unlikely that a reader.ReadLine() would generate a null but 
        // just in case it does, use == null instead of .Equals() method. When content[i] == null, you cannot use .Equals() method since nulls dont have Equals method.
        if (content[i].Equals(null))
        {
            return "content [" + i + "] returns null";
        }

        // If you have checked that content[i] is Not empty space or null, you might 
        //end up with newCon == null if Deserialization fails. Cover this around try / catch.
        newCon = JsonSerializer.Deserialize<Request_Template>(content[i]);

        // If deserialization fails, this will throw NRE because nulls wont have 
        // Sender as a property or array. Check if newCon == null or not. Also, 
        // check if Sender was correctly initialized as an array/list.. as it could 
        // be null. nulls wont have Contains as a method.
        if (newCon.Sender.Contains(myAccount.Username))
        {
            newCon.Sender = "Me";
        }

        string sender = newCon.Sender;
        string message = newCon.Message;

        // This should work correctly as its dependent on content.length. If the 
        // loop is happening, then there is at least one final element that can be updated.
        final[i] = sender + ":\t" + message;
    }

    string nFinal = string.Concat(final);

    Thread.Sleep(10);
    return nFinal;
}