MVC List Error List<Model>

Question

I'm using foreach to transfer data from list to another but when adding value updated automatically to last value added. For example:

list1 = [1,2,3]
list2 = new List<Model>()

foreach(var item in list1) {
   list2.Add(item)
}

the result in list2 is [ 3, 3, 3]

Actually example is below :

var _sizes = new List<ProductsSize>();
var _size = new ProductsSize();

if (model.Dynamic_ProductsSize.Count > 0)
{
    foreach (var item in model.Dynamic_ProductsSize)
    {
        _size.SizeId = item;
        _sizes.Add(_size);
    }
}
model.ProductsSize = _sizes.ToList();

I need to know why it only takes the last item and what is the solution for this case

Because you are adding the same object each time. You need to declare var _size = new ProductsSize(); inside the foreach loop — user3559349
– user3559349, Commented Aug 3, 2018 at 11:40

David · Accepted Answer · 2018-08-03 11:49:48Z

3

You only have one ProductsSize object:

var _size = new ProductsSize();

And you keep modifying that same object. All references to that object, including any list elements it's been added to, get updated when you modify that one object.

Instead, create your new object in the loop:

foreach (var item in model.Dynamic_ProductsSize)
{
    var _size = new ProductsSize();
    _size.SizeId = item;
    _sizes.Add(_size);
}

That way each element in the list is a new object instead of the same object added multiple times.

Side note, you have a few things in the code which aren't necessary. Checking the length before the loop, for example, as well as converting a list to a list at the end.

In fact, I imagine all of the code shown can be shortened to simply this:

model.ProductsSize = model.Dynamic_ProductsSize.Select(p => new ProductsSize { SizeId = p }).ToList();

In which case you're also just converting one model property to another model property. Why not put this logic in the model itself and skip the whole thing?

public IEnumerable<ProductsSize> ProductsSize
{
    get { return this.Dynamic_ProductsSize.Select(p => new ProductsSize { SizeId = p });
}

Unless there's a particular reason you want the same data twice in two different properties that isn't clear from this code, having one set of data and just different views/calculations/etc. of that data is often preferred.

edited Aug 3, 2018 at 11:49

answered Aug 3, 2018 at 11:41

David

221k42 gold badges245 silver badges337 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Camilo Terevinto Over a year ago

It should also be noted that the ToList() call at the end (model.ProductsSize = _sizes.ToList();) is completely unnecessary

David Over a year ago

@CamiloTerevinto: Indeed. As is the check for if (model.Dynamic_ProductsSize.Count > 0). And the loop body itself can be shortened to one line. Now that I think about it, all of the code shown can be just a single LINQ projection...

Postlagerkarte · Accepted Answer · 2018-08-03 11:43:33Z

0

Create a new object before adding it to the list. You can use the object initializer syntax to keep it concise:

 if (model.Dynamic_ProductsSize.Count > 0)
{
    foreach (var item in model.Dynamic_ProductsSize)
    {
        _sizes.Add(new ProductsSize(){SizeId = item});
    }
}

answered Aug 3, 2018 at 11:43

Postlagerkarte

7,1775 gold badges39 silver badges54 bronze badges

Collectives™ on Stack Overflow

MVC List Error List<Model>

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related