tags:

views:

1359

answers:

5

Short question: How can I modify individual items in a List? (or more precisely, members of a struct stored in a List?)

Full explanation:

First, the struct definitions used below:

public struct itemInfo
{
  ...(Strings, Chars, boring)...
  public String nameStr;
  ...(you get the idea, nothing fancy)...
  public String subNum; //BTW this is the element I'm trying to sort on
}

public struct slotInfo
{
  public Char catID;
  public String sortName;
  public Bitmap mainIcon;
  public IList<itemInfo> subItems;
}

public struct catInfo
{
  public Char catID;
  public String catDesc;
  public IList<slotInfo> items;
  public int numItems;
}

catInfo[] gAllCats = new catInfo[31];

gAllCats is populated on load, and so on down the line as the program runs.

The issue arises when I want to sort the itemInfo objects in the subItems array. I'm using LINQ to do this (because there doesn't seem to be any other reasonable way to sort lists of a non-builtin type). So here's what I have:

foreach (slotInfo sInf in gAllCats[c].items)
{
  var sortedSubItems =
    from itemInfo iInf in sInf.subItems
    orderby iInf.subNum ascending
    select iInf;
  IList<itemInfo> sortedSubTemp = new List<itemInfo();
  foreach (itemInfo iInf in sortedSubItems)
  {
    sortedSubTemp.Add(iInf);
  }
  sInf.subItems.Clear();
  sInf.subItems = sortedSubTemp;   // ERROR: see below
}

The error is, "Cannot modify members of 'sInf' because it is a 'foreach iteration variable'".

a, this restriction makes no sense; isn't that a primary use of the foreach construct?

b, (also out of spite) what does Clear() do if not modify the list? (BTW, the List does get cleared, according to the debugger, if I remove the last line and run it.)

So I tried to take a different approach, and see if it worked using a regular for loop. (Apparently, this is only allowable because gAllCats[c].items is actually an IList; I don't think it will allow you to index a regular List this way.)

for (int s = 0; s < gAllCats[c].items.Count; s++)
{
  var sortedSubItems =
    from itemInfo iInf in gAllCats[c].items[s].subItems
    orderby iInf.subNum ascending
    select iInf;
  IList<itemInfo> sortedSubTemp = new List<itemInfo>();
  foreach (itemInfo iInf in sortedSubItems)
  {
    sortedSubTemp.Add(iInf);
  }
                    //NOTE: the following two lines were incorrect in the original post
  gAllCats[c].items[s].subItems.Clear();
  gAllCats[c].items[s].subItems = sortedSubTemp;   // ERROR: see below

}

This time, the error is, "Cannot modify the return value of 'System.Collections.Generic.IList.this[int]' because it is not a variable." Ugh! What is it, if not a variable? and when did it become a 'return value'?

I know there has to be a 'correct' way to do this; I'm coming to this from a C background and I know I could do it in C (albeit with a good bit of manual memory management.)

I searched around, and it seems that ArrayList has gone out of fashion in favor of generic types (I'm using 3.0) and I can't use an array since the size needs to be dynamic.

Thanks!

+2  A: 

The problem you are having in your foreach is that structs are value types, and as a result, the loop iteration variable isn't actually a reference to the struct in the list, but rather a copy of the struct.

My guess would be the compiler is forbidding you change it because it most likely would not do what you expect it to anyway.

subItems.Clear() is less of a problem, because altho the field may be a copy of the element in the list, it is also a reference to the list (shallow copy).

The simplest solution would probably be to change from a struct to a class for this. Or use a completely different approach with a for (int ix = 0; ix < ...; ix++), etc.

jerryjvl
Is there any way to do a foreach() with a reference type?Also, re: the "for" approach, I tried that, as noted... different error, though.
andersop
Classes are reference types... classes can be used in a foreach... so yes... (not sure what brought that question on) ... and sorry about insufficient detail about the 'for' approach... I must not have paid enough attention to your question. I see Fredrik already clarified though.
jerryjvl
+7  A: 

Looking at the for-loop approach, the reason (and solution) for this is given in the documentation for the compilation error:

An attempt was made to modify a value type that is produced as the result of an intermediate expression but is not stored in a variable. This error can occur when you attempt to directly modify a struct in a generic collection.

To modify the struct, first assign it to a local variable, modify the variable, then assign the variable back to the item in the collection.

So, in your for-loop, change the following lines:

catSlots[s].subItems.Clear();
catSlots[s].subItems = sortedSubTemp;   // ERROR: see below

...into:

slotInfo tempSlot = gAllCats[0].items[s];
tempSlot.subItems  = sortedSubTemp;
gAllCats[0].items[s] = tempSlot;

I removed the call to the Clear method, since I don't think it adds anything.

Fredrik Mörk
Awesome, that works. Many thanks!
andersop
thanks for pointing this out. I knew about this as far as modifying property of a struct property, but I didn't know it applied to generic collections. I just assumed that the List itself was a class, so it's indexer would return a reference TO the struct. I imagine that's how it works with non-generic collections. thanks again
LoveMeSomeCode
Just found this solution. Works - thanks!
Chris Smith
+1  A: 

The foreach loop doesn't work because sInf is a copy of the struct inside items. Changing sInf will not change the "actual" struct in the list.

Clear works because you aren't changing sInf, you are changing the list inside sInf, and Ilist<T> will always be a reference type.

The same thing happens when you use the indexing operator on IList<T> - it returns a copy instead of the actual struct. If the compiler did allow catSlots[s].subItems = sortedSubTemp;, you'll be modifying the subItems of the copy, not the actual struct. Now you see why the compiler says the return value is not a variable - the copy cannot be referenced again.

There is a rather simple fix - operate on the copy, and then overwrite the original struct with your copy.

for (int s = 0; s < gAllCats[c].items.Count; s++)
{
            var sortedSubItems =
                            from itemInfo iInf in gAllCats[c].items[s].subItems
                            orderby iInf.subNum ascending
                            select iInf;
            IList<itemInfo> sortedSubTemp = new List<itemInfo>();
            foreach (itemInfo iInf in sortedSubItems)
            {
                            sortedSubTemp.Add(iInf);
            }
            var temp = catSlots[s];
            temp.subItems = sortedSubTemp;
            catSlots[s] = temp;
}

Yes, this results in two copy operations, but that's the price you pay for value semantics.

Senthil Kumar
Ahh, the indexing[] is actually a copy as well, but only if it's on a list -- for an array, it's a value type. Good to know, thanks.
andersop
+1  A: 

The two errors you specified have to do with the fact that you are using structs, which in C# are value types, not reference types.

You absolutely can use reference types in foreach loops. If you change your structs to classes, you can simply do this:

    foreach(var item in gAllCats[c].items)
    {
        item.subItems = item.subItems.OrderBy(x => x.subNum).ToList();
    }

With structs this would need to change to:

    for(int i=0; i< gAllCats[c].items.Count; i++)
    {
        var newitem = gAllCats[c].items[i];
        newitem.subItems = newitem.subItems.OrderBy(x => x.subNum).ToList();
        gAllCats[c].items[i] = newitem;
    }

The other answers have better information on why structs work different than classes, but I thought I could help with the sorting part.

NerdFury
+1  A: 

If subItems was changed to a concrete List instead of the interface IList, then you'd be able to use the Sort method.

public List<itemInfo> subItems;

So your whole loop becomes:

foreach (slotInfo sInf in gAllCats[c].items)
    sInf.subItems.Sort();

This won't require the contents of the struct to be modified at all (generally a good thing). The struct's members will still point to exactly the same objects.

Also, there are very few good reasons to use struct in C#. The GC is very, very good, and you'd be better off with class until you've demonstrated a memory allocation bottleneck in a profiler.

Even more succinctly, if items in gAllCats[c].items is also a List, you can write:

gAllCats[c].items.ForEach(i => i.subItems.Sort());

Edit: you give up too easily! :)

Sort is very easy to customise. For example:

var simpsons = new[]
               {
                   new {Name = "Homer", Age = 37},
                   new {Name = "Bart", Age = 10},
                   new {Name = "Marge", Age = 36},
                   new {Name = "Grandpa", Age = int.MaxValue},
                   new {Name = "Lisa", Age = 8}
               }
               .ToList();

simpsons.Sort((a, b) => a.Age - b.Age);

That sorts from youngest to oldest. (Isn't the type inference good in C# 3?)

Daniel Earwicker
Then I'd have to define what it means to sort those objects -- I really want to sort on one specific item. I suppose this would involve overloading the Sort() method for itemInfo, but the docs are really terrible about that kind of thing. I actually made gAllCats an array because I couldn't figure out how to manipulate it if I used a List. Ugh.... Anyway, thanks.
andersop