0

So I've got this class with two lists right now. One is getting initialized and one is not and I don't understand why. someList is just there, because I thought I am too stupid to initialize a list, but someList works just fine.

Here's my class:

using System;
using System.Collections.Generic;
using System.IO;
using Newtonsoft.Json;

class Calendar
{
    static List<CalendarNode> _calendar = new List<CalendarNode>();
    static List<int> someList = new List<int>();

    public static int addNode(CalendarNode node)
    {
        if (_calendar != null)
        {
            foreach (CalendarNode item in _calendar)
            {
                if (item.Username == node.Username)
                {
                    return 1;
                }

            }

        }
        someList.Add(1);        // works like a charm
        _calendar.Add(node);    // throws Error Object reference not set to an instance of an object
    }
}

What am I doing wrong?

Thanks for your help

edit1:

Regex pattern = new Regex(@"\.0000");
        DateTime date;
        IUserMessage message;
        CalendarNode node = new CalendarNode();

        try
        {
            if (pattern.Match(birthday).Success)
            {
                Regex replace = new Regex(@"\d{4}");
                birthday = replace.Replace(birthday, "1900");
                date = DateTime.ParseExact(birthday, "dd.MM.yyyy", System.Globalization.CultureInfo.InvariantCulture);
            }
            else
            {
                date = DateTime.ParseExact(birthday, "dd.MM.yyyy", System.Globalization.CultureInfo.InvariantCulture);
            }

            node = new CalendarNode(Context.User.Mention, date);
        }
        catch (Exception)
        {
            message = await Context.Channel.SendMessageAsync($"Probably invalid date! Use format: \"dd.mm.yyyy\"\nIf the error persists, contact Nigolasy");
        }
        int status = 2;
        if (node != null)
        {
            status = Calendar.addNode(node); 
        }

This is the code where I call my addNode.

And my CalendarNode class, in case it is important

class CalendarNode
{
    string username;
    DateTime birthdate;

    public CalendarNode(string username, DateTime birthdate)
    {
        this.username = username;
        this.birthdate = birthdate;
    }
    public CalendarNode(){ }

    public string Username { get { return username; } }
    public DateTime Birthdate { get { return birthdate; } }
}
Nicola K
  • 21
  • 2
  • 4
    As for me, this code looks good. Is there any other relevant code? P.S. Of course, assuming that it throws exception at the *specified line*. – Yeldar Kurmangaliyev Jul 18 '18 at 13:48
  • Your `addNode` method doesn't do any null checking for the `node` argument. – Johnathan Barclay Jul 18 '18 at 13:49
  • 1
    parameter node might be null. – Manpreet Singh Dhillon Jul 18 '18 at 13:49
  • Ah, are you sure the node in this faulting line is not null? – BugFinder Jul 18 '18 at 13:49
  • 5
    Other than the `addNode` method not returning anything (which means the code wouldn't compile) I see nothing wrong here. Is this really your code? – DavidG Jul 18 '18 at 13:49
  • 1
    Are you *sure* it's that line that's throwing the exception? Because if `node` is null and `_calendar` is not null (which it won't be) and if it is also not empty, then the `if (item.Username == node.Username)` will throw a null reference exception when you access `node.Username` – Matthew Watson Jul 18 '18 at 13:52
  • Why are you checking `if (_calendar != null)`? In the code as it is, `_calendar` should never be null. Are you at some point setting it to null? if not, make it `static readonly List _calendar = new List();`. The added `readonly` will prevent any piece of code to set `_calendar` to null, making the ckeck unnecessary. – Corak Jul 18 '18 at 13:53
  • You will get this exception if there are any entries in the Calendar list & node is null OR if any entry in the Calendar list is null. You need to check for node being null & also _item_ being null as the first statement in the foreach loop. – PaulF Jul 18 '18 at 14:08
  • The code looks fine. Even if `node` is `null` it would work - you can add `null` to a `List<>`. Is that definitely the line it's throwing on? – Stevo Jul 18 '18 at 14:16
  • @SteveJ: with this code a null node can only be added as the very first entry - subsequent attempts to add nodes, null or otherwise, will result in an exception because _item_ is null. – PaulF Jul 18 '18 at 14:18
  • So befor calling addNode I check if the node is null. My node definitly is not null. I used break points to figure out when exactly my null exeption occurs and it is the _calendar.Add(node). – Nicola K Jul 18 '18 at 14:46
  • @Corak Thats for future content when I am going to delete nodes. You can ignore this part for now as I am only changing _calendar in addNode at the moment. I just copied everything I have – Nicola K Jul 18 '18 at 14:49
  • 1
    Why would deleting nodes need `_calendar` to not be `readonly`? Anyways, if you are *sure* the exception happens at that exact line, the exception can only be caused by `_calendar` being null (you can see that with a breakpoint to make sure `_calendar` is null). Now that raises the question: at what point are **you** setting it to null? The code you posted doesn't show that. With the code you posted, `_calendar` **cannot** be null. So you're hiding something from us. – Corak Jul 18 '18 at 15:00
  • When you checked with the debugger & the exception occurred at "_calendar.Add(node)"_ had you previously added nodes. Did you check if _"_calendar"_ was equal to null. – PaulF Jul 18 '18 at 15:00
  • @corak Okay, so I just made a full code search with Visual Studio for the phrase _calendar and I got not other results than those I posted. – Nicola K Jul 18 '18 at 15:04
  • @PaulF When I debug the code, my breakpoint is at someList.Add(1). If I hover over _calendar at this point, it is null. – Nicola K Jul 18 '18 at 15:05
  • Is this the first node being added to the list? – PaulF Jul 18 '18 at 15:11
  • 1
    It is the kind of code that is likely to run on a worker thread. The List class is not thread-safe, `lock` is required to protect it. – Hans Passant Jul 18 '18 at 15:19
  • @PaulF It should be. I am just trying to figure out if it acually is. @HansPassant Yes, the code acually is running on an own thread. And tbh it is the first time I hear of `lock`. I will read about it and try it out. Thanks for the hint! – Nicola K Jul 18 '18 at 15:25
  • For debugging purposes I would add a static constructor to Calendar & initialise the two lists there - then a breakpoint could be set to ensure the initialisation is done. Setting the fileds to readonly as Corak suggested would also rule out accidentally setting null values (or hiding the fields behind a property with only a get). – PaulF Jul 18 '18 at 15:39
  • @HansPassant - just to appease my mind, even in threaded context, static construction and initialisation of static fields is guaranteed to take place *before* any static method can be called, right? – Corak Jul 18 '18 at 15:55
  • Yes, no reason to worry about that specific detail. – Hans Passant Jul 18 '18 at 17:38

2 Answers2

1

So, I am acually ashamed.

After debugging EVERYTHING from the literal begining of my code, I found a load function, that was not yet supposed to be there, that was setting my _calendar with an empty JSON string ofc to an empty list. I must have been half asleep when I wrote it that I didn't put it in my class...

I have no clue why Visual Studio did not show me this part, when I was searching for _calendar earlier. Maybe because I didn't have the startup file open?

Thank's anyways for your help and time.

(I will still look into the lock part. Looks very important!)

Nicola K
  • 21
  • 2
  • 1
    Hiding the fields behind properties would have helped here too - they allow breakpoints to be set on the property being changed as managed code does not allow for data breakpoints. – PaulF Jul 18 '18 at 16:00
  • See also: [lock](https://learn.microsoft.com/dotnet/csharp/language-reference/keywords/lock-statement) and [thread synchronization](https://learn.microsoft.com/dotnet/csharp/programming-guide/concepts/threading/thread-synchronization) – Corak Jul 18 '18 at 16:06
0

Your app is adding nodes to a list that is null because it is outside of the check for null.

public static int addNode(CalendarNode node)
{
    if (_calendar != null)
    {
        foreach (CalendarNode item in _calendar)
        {
            if (item.Username == node.Username)
            {
                return 1;
            }

        }
             _calendar.Add(node);
    }

}
Chad
  • 1,512
  • 1
  • 16
  • 40
  • This is not what that if is supposed to do. If the User is found in the Calendar, it should not be added. That's why I return 1 and not 0. – Nicola K Jul 18 '18 at 14:57
  • I think the idea of the code is that it does not add duplicate entries to the list - so it loops though the current list & exits if it finds a match. With this code a node could never be added. – PaulF Jul 18 '18 at 14:57
  • @NicolaK - Updated the answer with the better logic for what you want to do – Chad Jul 18 '18 at 15:22
  • Well that would avoid the exception - but it does not answer why _"_calendar"_ is null in the first place. It is initialised to a new instance of the correct type of list & OP has checked the code doesn't seem to set it to null anywhere. "somelist" is initialised in the same way & is not null. – PaulF Jul 18 '18 at 15:28