Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C# List<T>.Add not adding properly

Tags:

c#

Using .Add to add an instance of a class to a generic list is not working.

To illustrate the problem, here are two simple example classes:

public class WorkOrder
{
    private List<Note> _Notes;
    public List<Note> Notes
    {
        get
        {
            return _Notes ?? new List<Note>();
        }
        set
        {
            _Notes = value;
        }
    }
}

public class Note
{
    public string NoteText { get; set; }
    public System.DateTime Time { get; set; }
    public string User { get; set; }
}

You may notice the coding in get on the WorkOrder.Notes property. I put this in so the property wouldn't be initialized with a null value (ref an answer to another question I posted on SO here).

To utilize these classes:

public void Test()
{
    WorkOrder tempWorkOrder = new WorkOrder();
    Note tempNote = new Note()
    {
        User = "Aaron",
        Time = DateTime.Now,
        NoteText = "Work Order pulled from CSV Excel report."
    };
    tempWorkOrder.Notes.Add(tempNote);
}

I would expect the last line in Test() to add tempNote to the list of Note in tempWorkOrder. However, tempWorkOrder.Notes is null after this line completes. No errors or exceptions are thrown.

I'm using VS2013 Express.

What am I doing wrong?

like image 490
Aaron Thomas Avatar asked Feb 27 '15 14:02

Aaron Thomas


4 Answers

private List<Note> _Notes;
public List<Note> Notes
{
    get
    {
        return _Notes ?? new List<Note>();
    }
    set
    {
        _Notes = value;
    }
}

The get is wrong. It should be:

    get
    {
        if (_Notes == null) {
            _Notes = new List<Note>();
        }
        return _Notes;
    }

because otherwise you don't save the new List<Note>() you created and every time you use the get you recreate it (the get returns a new List<Note>() but doesn't modify _Notes, so every get checks _Notes, see it's null and return a new List<Note>())

Note that if you hate the world (and your fellow programmers) you can compact the get to:

return _Notes ?? (_Notes = new List<Note>());

(see Ternary/null coalescing operator and assignment expression on the right-hand side?) I don't hate enough the world (and my fellow programmers) to do it :-)

like image 89
xanatos Avatar answered Nov 12 '22 12:11

xanatos


You have not created the list yet there. You need to add a constructor to the WorkOrder as you cannot add to a collection that does not exist. This way, whenever you create a Work Order, you will have an empty list in the `_Notes' field.

It would look something like this:

WorkOrder(){
    _Notes = new List<Note>();
}
like image 29
David Watts Avatar answered Nov 12 '22 11:11

David Watts


You never assign _Notes

Do this instead

    private List<Note> _Notes;
    public List<Note> Notes
    {
        get
        {
            if(_Notes == null)
                 _Notes = new List<Note>();
            return _Notes;
        }
        set
        {
            _Notes = value;
        }
    }
like image 1
Jeffrey Wieder Avatar answered Nov 12 '22 11:11

Jeffrey Wieder


You're not initializing _Notes.

So while you get a List<Note> back when _Notes is null, it is not assigning the object to _Notes. Each time you access the public property, it is returning a different List<Note> which is why the Add() call appears to not work.

You should rather use:

get 
{ 
  if (_Notes == null) 
     _Notes = new List<Note>(); 
  return _Notes; 
}
like image 1
toadflakz Avatar answered Nov 12 '22 11:11

toadflakz