CODEWITHSUNDEEP
c#listgetter
Gerard
Gerard

Reputation: 13397

Add item to a list, without effect

In a C# class I have a list and two different getters for the list:

private List<A> a;
public List<A> EveryA
   {
      get
      {
          if (a == null) a = new List<A>();
          return a;
      }
   }
public List<A> FilteredA
   {
       get
       {
          return EveryA.FindAll(a => a.IsInFilter);
       }
    }

Now my question is: how about the syntax FilteredA.Add(this);?
It compiles and runs but it cannot add any item to any list.
Should a better compiler have to notify the (small) problem?

Upvotes: 0

Views: 326

Answers (5)

Jodrell
Jodrell

Reputation: 35716

public List<A> FilteredA returns some output of the FindAll method, as a List<A>. This will not be the same object as EveryA so when it goes out of scope your addition will be lost.

Upvotes: 2

Kristoffer
Kristoffer

Reputation: 834

It's not really a compiler issue - since the code is valid it will compile just fine. The problem is more on a code quality level. To catch something like this, you could use a tool like FxCop to analyze your code.

Both methods can be seen as query methods. You should not expose the result as a List, but rather an IEnumerable or A[]. If you want to add an item to the list, do so with an Add method.

private List<A> items = new List<A>();

public IEnumerable<A> EveryA
{
  get { return items; }
}

public IEnumerable<A> FilteredA
{
  get { return items.Where(item => item.IsInFilter); }
}

public void AddItem(A item)
{
  items.Add(item);
}

Upvotes: 1

Daren Thomas
Daren Thomas

Reputation: 70314

They are not the same list. This is not something the compiler can check for you, since the compiler can't really read your mind. Check the documentation for List<T>.FindAll

The result is a list, but it isn't the same list (how could it be? your original list isn't filtered!).

You should be able to add items to the list returned by FilteredA, except they won't show up in a.

I suggest you use LINQs Where instead, returning an IEnumerable<T>. That way, it is obvious that the result of FilteredA shouldn't be changed, only iterated over:

public IEnumerable<A> FilteredA
{
    get { return EveryA.Where(a => a.IsInFilter); }
}

Upvotes: 5

phoog
phoog

Reputation: 43046

FindAll returns a new list. You're adding the new item to the new list but not retaining a reference to the new list, I suppose. The semantics would be clearer if the filtered list came from a method rather than a property.

Upvotes: 3

Daniel Hilgarth
Daniel Hilgarth

Reputation: 174299

No. Why should it notify you about this? It is completely ok.
FilteredA doesn't return a but a new instance of a List<A>.
FilteredA.Add(this); adds this to this new instance.

See this code:

var filteredA = FilteredA;
int count1 = filteredA.Count;
filteredA.Add(this);
int count2 = filteredA.Count;
Assert.AreEqual(count1 + 1, count2);

This shows, that the new item IS added to the list. But to that new instance that is independent of the list inside your class.

Upvotes: 3

Related Questions