0

I have a for loop which invokes a method within it. I add the return of this method to a list however I am getting duplicates in the list (the last return from the method call is all the items in the list). Presumably this is because the result object is the same instance. Is there a way around this?

IList<CarResult> carResults = new List<CarResult>();

for (int i = 0; i < cars.Count(); i++)
{              
   result = calculation.RunForCar(
                engineSize[i],
                yearOfManufacture[i],

   carResults.Add(result);
}

   return carResults;
}
3
  • 2
    Can you post your RunForCar method as well and in the code above I think you have a ) typo. Commented Sep 26, 2016 at 11:31
  • The code provided in the question seems to be okay. You could simply the code by using the yield return statement (depending on the return type of your method). Please post the method signature and the RunForCar method as well. Commented Sep 26, 2016 at 11:33
  • 1
    Based on the behavior you are describing I guess you are re-using the result object inside RunForCar() instead of newing it up. Commented Sep 26, 2016 at 11:51

3 Answers 3

3

I'm going to make a qualified guess and try to explain what's going on, without knowing exactly what's happening in your RunForCar().

Presumably this is because the result object is the same instance.

Probably yes.

Here's an example. It will not create new instances of Foo, but re-use the same instance over and over. So every time the name changes it changes the name on the reference. The list itself only contains the references, and therefore all the items in the list will be changed if you change the name on the reference.

var list = new List<Foo>();
var result = new Foo();
for(int i = 0; i < 5; i++)
{
    result.Name = i.ToString();
    list.Add(result);
}

foreach (var foo in list)
{
    Console.WriteLine(foo.Name);
}

Output:

4
4
4
4
4

If we instead do like the code below, we assign result to a new reference, and then we leave the existing references untouched.

var list = new List<Foo>();
var result = new Foo();
for(int i = 0; i < 5; i++)
{
    result = new Foo()
    {
        Name = i.ToString()
    };
    result.Name = i.ToString();
    list.Add(result);
}

foreach (var foo in list)
{
    Console.WriteLine(foo.Name);
}

Output:

0
1
2
3
4

Is there a way around this?

Yes, you can simply create a new instance of result for every loop. Without knowing more about either CarResult or RunForCar I cannot say when it's best to create the new instance. But here's an example:

IList<CarResult> carResults = new List<CarResult>();
for (int i = 0; i < cars.Count(); i++)
{              
    result = new CarResult();
    result = calculation.RunForCar(
        engineSize[i],
        yearOfManufacture[i]); // Fixed type-o?

    carResults.Add(result);
}
return carResults;

Alternatively you can have a local variable inside the loop.

IList<CarResult> carResults = new List<CarResult>();
for (int i = 0; i < cars.Count(); i++)
{              
    var result = new CarResult(); // Will not be accessible outside of loop.
    result = calculation.RunForCar(
        engineSize[i],
        yearOfManufacture[i]); // Fixed type-o?

    carResults.Add(result);
}
return carResults;
Sign up to request clarification or add additional context in comments.

Comments

0

If the result is the same instance, you need to replace IList with HashSet

Comments

0

You need to create a new instance or result object on every pass of the loop in order to avoid adding it by reference to carResults list. Oterwise, all items in carResults will hold the reference to the same object which will contain the data from the last loop cycle.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.