0

I am getting an 'Object reference not set to an instance of an object' error and i cannot workout why as i am declaring and creating the object globally.

The exception is firing when i try to add the first item to a dictionary. Heres my code:

//declared at the top of my page globally
Dictionary <String, MyCustomType> MyDictionary = new Dictionary();
MyCustomType myCustomType = new MyCustomType();

//Then later in a (private void) method:
//First part of if statement checks if we already have anything in there 
//and if the key already exists. If so, it overwrites it

if (MyDictionary != null && MyDictionary.ContainsKey(some_string)
{
    //  replace the item in the dictionary
}
// if the dictionary is null or the key isnt already in the dictionary.  
// This is where it is throwing the exception
else 
{
    MyDictionary.add(some_string, myCustomType)
}

When debugging through, it does say the MyDictionary is null, but this is to be expected as i have nothing in there yet, and the act off the add in the else statement puts something in there, so im really not sure why its throwing an exception here. The debug also shows that some_string and myCustomType has the values i expect them to have. Can anyone help?

Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
Ricardinho
  • 53
  • 2
  • 10
  • 1
    As you own comment states _**if the dictionary is null** or the key isnt already in the dictionary_ then add key value – dkozl May 15 '15 at 09:55
  • Yeah - but that is where it is throwing the exception. When i try to add the item – Ricardinho May 15 '15 at 09:56
  • 1
    You code contains errors (e.g. `new Dictionary()`, non-matching curly braces). It would not compile. – stakx - no longer contributing May 15 '15 at 09:57
  • @Ricardinho yes, because it will go to else part also if `MyDictionary` is null and there you just add an item without checking if it's null – dkozl May 15 '15 at 09:57
  • It's a logic problem... if MyDictionary is null then you fall to the else, and try adding to a null dictionary. – Paul Zahra May 15 '15 at 09:58
  • try to replace your `new Dictionary()` with `new Dictionary()` – Julien May 15 '15 at 10:00
  • Thanks all for the comments. Got a lot to go on here! :) – Ricardinho May 15 '15 at 10:05
  • it's a good practice to define collections as readonly fields when possible, e.g: `private readonly Dictionary MyDictionary = new Dictionary();` you won't be able to set it to null, but you will find out, that you don't need to. – Liero May 15 '15 at 10:06
  • This could have been closed as a duplicate as it was a simple case of assuming something wasn't null when it clearly was. Program logic changes, but the problem doesn't. – Adam Houldsworth May 15 '15 at 11:03

4 Answers4

2

Keep it simple:

if (MyDictionary == null) MyDictionary = new Dictionary<string, CustomType>();
MyDictionary[some_string] = myCustomType;
Liero
  • 25,216
  • 29
  • 151
  • 297
  • Thanks - im going to rework my code to always assign to a dictionary this way – Ricardinho May 15 '15 at 10:06
  • Even better solution would be to declare MyDictionary as readonly field, so it will never be null. than you dont need to do null check. – Liero May 15 '15 at 10:09
  • FYI - this method worked. I still have no idea how/why/where my dictionary is being set to null, but it is somewhere and this at least caters for that scenario – Ricardinho May 15 '15 at 10:10
  • just a tip for you: To find out, where your dictionary is being set to null, convert the field to property with backing field and add breakpoint to setter. breakpoint will be hit anytime somebody is trying to set the value and you can see it in callstack window – Liero May 15 '15 at 10:12
2

Even if you are sure that you have no null references, there must be one; otherwise you would not get a NullReferenceException. The runtime is usually correct about this.

  • Have you made sure that the code initializing myDictionary runs at all?
  • Have you ensured that the initialization of myDictionary always runs before your private method that uses it?

Now on to something else:

myDictionary[key] = value;

will work in both cases, i.e. when the dictionary does not yet have an entry for key, but also when there is already an entry for key.

Using [] has the advantage over your current two-step process (first check using ContainsKey, then Add or update) of being more atomic; there is less risk of another thread interfering with your dictionary between the two steps. (Though, truth be told, if that is an issue at all, you might be better off using ConcurrentDictionary.)

stakx - no longer contributing
  • 83,039
  • 20
  • 168
  • 268
1

You need to test if MyDictionary is null and instantiate it if it is, before calling MyDictionary.add(...

The line:

Dictionary <String, MyCustomType> MyDictionary = new Dictionary();

won't compile, so I'm not sure what you really have in your code.

You'll need to change the code to something like:

if (MyDictionary != null && MyDictionary.ContainsKey(some_string)
{
    ...
}
else
{
    if (MyDictionary == null)
    {
        MyDictionary = new Dictionary<String, MyCustomType>(); 
    }
    MyDictionary.Add(some_string, myCustomType);
}
David Arno
  • 42,717
  • 16
  • 86
  • 131
0

I think you should remake your if like this

if (MyDictionary != null)
{
    if(MyDictionary.ContainsKey(some_string)
    {
        //  replace the item in the dictionary
    }
     // if the dictionary is null or the key isnt already in the dictionary.      This is where it is throwing the exception
    else 
    {
        MyDictionary.add(some_string, myCustomType)
    }
}
csharpwinphonexaml
  • 3,659
  • 10
  • 32
  • 63