List.Add seems to be duplicating entries. What's wrong?

Question

I have a class like this:

public class myClass
{
  public List<myOtherClass> anewlist = new List<myOtherClass>;

  public void addToList(myOtherClass tmp)
  {
    anewList.Add(tmp);
  }

}

So I call "addToList" a hundred times, each adding a unique item to the list. I've tested my items to show that before I run the "addToList" method, they are unique. I even put a line in to test "tmp" to make sure it was what I was expecting.

However, when I do this (lets say myClass object is called tmpClass):

int i = tmpClass.anewList.Count();
for (int j = 0; j<i; j++)
{
   //write out each member of the list based on index j...
}

I get the same exact item, and it's the last one that was written into my list. It's as if when I add, I'm overwriting the entire list with the last item I've added.

Help? This makes no sense. I've also tried List.Insert, where I'm always inserting at the end or at index 0. Still no dice. Yes, I'm doubly source my indexing is correct and when I do my test I'm indexing through each of the elements.

:)

UPDATE: Okay, I tried this and still had the same problem:

foreach(myOtherClass tmpC in tmpClass.anewList)
{    
    Console.WriteLine(tmpC.theStringInMyClass.ToString());
}

and still for each of the 100 items, I got the same string output... I'm sure I'm doing something completely stupid, but I don't know what yet. I'm still 100% sure that the right string is getting passed in to begin with.

-Adeena

Answer 1

Try iterating through the list using foreach rather than by index. I suspect that the problem is in the code that you have omitted from your example, not the list itself.

foreach (MyOtherClass item in tmpClass.anewList)
{
     Console.WriteLine( item );  // or whatever you use to write it
}

EDIT

Have you examined the list structure in the debugger to ensure that your unique items are actually added? Also, you might want to use .Count (the property) instead of .Count() (the extension method). The extension method may actually iterate over the list to count the methods, while the property is just looking up the value of a private variable that holds the count.

@James may be onto something here. If you are just changing the properties of the item you've inserted and reinserting it, instead of creating a new object each time, this would result in the behavior you are seeing.

Answer 2

Okay, I tried this and still had the same problem:

foreach(myOtherClass tmpC in tmpClass.anewList)
{
    Console.WriteLine(tmpC.theStringInMyClass.ToString());
}

and still for each of the 100 items, I got the same string output... I'm sure I'm doing something completely stupid, but I don't know what yet. I'm still 100% sure that the right string is getting passed in to begin with.

-Adeena

Answer 3

Also note that when you use the exact form:

 for (int j = 0; j < tmpClass.anewList.Count(); j++)

The C# compile preforms a special optimization on the loop. If you vary from the syntax (e.g. by pulling the Count property out of the loop into a separate varaible, as you did in you example), the compile skips that optimization.

It won't affect what is displayed, but it will take longer.

Answer 4

In this case, it would likely be helpful to see how you are validating each item to be sure that the items are unique. If you could show the ToString() method of your class it might help: you might be basing it on something that is actually the same between each of your objects. This might help decide whether you really are getting the same object each time, or if the pieces under consideration really are not unique.

Also, rather than accessing by index, you should use a foreach loop whenever possible.

Finally, the items in a list are not universally unique, but rather references to an object that exists elsewhere. If you're trying to check that the retrieved item is unique with respect to an external object, you're going to fail.

One more thing, I guess: you probably want to have the access on anewList to be private rather than public.

Answer 5

For debugging, to ensure that I was building up my list "uniquely", I did the following:

public class myClass
{
  public List<myOtherClass> anewlist = new List<myOtherClass>;

  public void addToList(myOtherClass tmp)
  {
    //A
    Console.WriteLine(" the input = "+tmp.theStringInMyClass.ToString());
    anewList.Add(tmp);
    int i = anewList.Count();
    //B
    Console.WriteLine(" the new list count = "+ i);
    Console.WriteLine(" the last entry =" +anewList.ElementAt(i-1).theStringInMyClass.ToString());
    if (i == 100) // just for debug, I know this is my last entry
    {
       foreach (myOtherClass tmpL in anewList)
       {
           //C
           Console.WriteLine( tmpL.theStringInMyClass.ToString());
       }
    }
  }

}

The output after comment "A" is what I expect - all the unique entries that are input. The output in the two lines after comment "B" is what I expect - i increases, and all the string output is the unique ones I expect. "C" is what's not right... it writes out the same entry, which happens to be the last one.

???

-Adeena

Answer 6

Given the signature of your addToList method:

public void addToList(myOtherClass tmp)
  {
    anewList.Add(tmp);
  }

Is is possible that in the consumer of that method, you aren't actually creating a new instance?

You said that you are calling addToList 100 times. Presumably, that is in a loop. At each loop iteration, you will need to create a new instance of "myOtherClass", otherwise, you'll just be updating the same object in memory.

For example, if you do the below, you will have 100 copies of the same object:

myOtherClass item = new myOtherClass();

for(int i=0; i < 100; i++)
{
  item.Property = i;
  addToList(item);
}

However, if your loop looks like the below, it will work fine:

myOtherClass item = null;
for(int i=0; i < 100; i++)
{
  item = new myOtherClass();
  item.Property = i;
  addToList(item);
}

Hope that helps!

Answer 7

Got it! Thank you James -

here's the stupid thing I did wrong:

I had:

myClass tmpClass = new myClass();
myOtherClass anewitem = new myOtherClass();
string tst = "";

for (int i = 0; i < 100; i++) 
{
    tst += "blah";
    anewitem.theStirngInMyClass = tst;
    tmpClass.AddToList(anewitem);
}

when I changed it to be this:

myClass tmpClass = new myClass();
string tst = "";

for (int i = 0; i < 100; i++) 
{
    myOtherClass anewitem = new myOtherClass()
    tst += "blah";
    anewitem.theStringInMyClass = tst;
    tmpClass.AddToList(tst);
}

All was well. I get it. :)

Thanks for the help guys!

-Adeena

Answer 8

Well, from I've read here, I supose your problem could be in adding items in list - are you sure, you're not adding the same reference again and again? That could be reason, why you have 100 "last items" in list.