problem assigning value to struct member in for each
kevinw
left-hand side of an assignment must be a variable, property or
indexer" when assigning a value to a struct member within a foreach
loop. Consider the following:
private struct Address
{
public string address1;
public string address2;
public string city;
public string state;
public string zip;
}
private void Main()
{
Address myAdrs = new Address();
myAdrs.address1 = "1234 main";
myAdrs.address2 = "";
myAdrs.city = "akron";
myAdrs.state = "oh";
myAdrs.zip = "44123";
ArrayList arlAddress = new ArrayList();
arlAddress.Add(myAdrs);
foreach(Address objAdr in arlAddress)
{
Address addr = objAdr;
addr.city = "cleveland"; //this line is ok
objAdr.city = "cleveland"; //compile error generated here
}
}
C# 2005 actually gives a more descriptive "Cannot modify members of
'objAdr' because it is a 'foreach iteration variable". Probably a
complicated reason for this behavior, involving boxing/unboxing,
reference types etc, but seems very strange.
Bruce Wood
that. Instead, you should use a class.
In order to understand the error message, you have to understand the
difference between structs and classes, between value semantics and
reference semantics. Please excuse me if you already know this: no
offense intended.
When you declare a struct in C#, you are declaring a _value_ type. That
means that every assignment of this type is a _copy_ operation, and
whenever you put it into a collection like an ArrayList, it is a _boxed
temporary copy_ that is placed in the ArrayList, not a reference to the
same thing you put in there. To illustrate this, here's an example:
Address myAdrs = new Address();
myAdrs.address1 = "1234 main";
myAdrs.address2 = "";
myAdrs.city = "akron";
myAdrs.state = "oh";
myAdrs.zip = "44123";
ArrayList arlAddress = new ArrayList();
arlAddress.Add(myAdrs);
Address otherAdrs = (Address)arlAddress[0];
otherAdrs.city = "cleveland";
// At this point, myAdrs.city is still "akron"!
...because when you put the Address into the ArrayList, .NET made a
boxed copy and put that copy in the list. When you got it back out
again, .NET unboxed the value and _copied_ it into otherAdrs, so myAdrs
and otherAdrs are distinct.
In fact, with value types, any two variables containing the same type
(for example, Address), are distinct, no matter how that value got
there. Changing one will _never_ change the other as a side-effect.
So, knowing this, let's look at your foreach loop:
foreach (Address objAdr in arlAddress)
{
Address addr = objAdr;
}
>From the compiler's point of view, there is a _fundamental_ difference
between objAdr and addr. First, notice that addr is a _copy_ of
objAddr, which is a _copy_ of the value in the ArrayList, because we're
dealing with value semantics. So, changing addr.city won't change
anything in the ArrayList. Guaranteed.
Knowing this, you can see that saying
foreach (Address objAdr in arlAddress)
{
Address addr = objAdr;
addr.city = "cleveland";
}
does nothing at all. It copies the Address value from objAdr into addr,
then changes its city to "cleveland", then throws that copy away for
the next trip around the loop. The entire loop is therefore a no-op. It
does nothing. It's equivalent to writing this:
ArrayList intList = new ArrayList();
intList.Add(2);
foreach (int i in intList)
{
int j = i;
j = j + 1;
}
The only hitch is that because you made a copy of objAdr in your
original example, the compiler didn't notice that you didn't do
anything with it. The compiler isn't smart enough to warn you that
you're just changing a value and then throwing it away.
However, in this case
foreach (Address objAdr in arlAddress)
{
objAdr.city = "cleveland";
}
the compiler _is_ smart enough to realize that the loop does nothing.
objAdr _is not_ the value stored in arlAddress. It's a _copy_ of the
value stored in arlAddress, created through an unbox operation. The
compiler knows this and sees that you're modifying the copy and doing
nothing else, so it complains.
The moral: don't use a struct in this situation. It's not what you
want. Use a class.
kevinw
types in this situation. I was aware of most of this but did need a
refresher. But it really doesn't answer my question as to why the
compiler considers my original issue an error. If every assignment of
a value type is a copy operation, then there should not be a problem
within the foreach loop of assigning a new value to a member of the
struct. Obviously the compiler is not making a copy, which seems to be
inconsistent behavior. Maybe that is done for performance reasons.
Keep in mind that my example was just that, an example, not a real
world situation. I can think of cases where using a struct instead of
an object in this situation makes sense. For instance, maybe I have a
struct with 100 members, and I want to make a copy of a struct instance
so that I can track the original values and modified values, which is
perfectly reasonable. If I used a class in this situation, I would
have to refer to all 100 members to make a copy of the object.
I think is is not appropriate to make general statements like "don't
use a struct in this situation, use a class". Leave that up to the
individual to decide based upon what they're trying to solve.
Bruce Wood
No, no. The compiler _is_ making a copy, which is precisely the
problem, viz:
1. Make a copy of the value in the ArrayList.
2. Change some property of the copy.
3. Throw the copy away.
4. Repeat for each item in array list.
It's a completely useless operation. The whole loop does nothing, so
the C# compiler designers created a compiler error saying that you
couldn't do this, not because the compiler couldn't generate the code
if it wanted to, but because in the situation you provided, the
compiler can detect that what you're trying to do will have no effect.
Copying the value into another variable you created only served to fool
the compiler and avoid the message. If the compiler were smarter it
would have (legitimately) complained about that, too.
Once you put a value into an ArrayList, the _only_ way to modify the
contents of the ArrayList is like this:
for (int i = 0; i < arlAddress.Count; i++)
{
Address addr = arlAddress[i];
addr.city = "cleveland";
arlAddress[i] = addr;
}
> I can think of cases where using a struct instead of
> an object in this situation makes sense. For instance, maybe I have a
> struct with 100 members, and I want to make a copy of a struct instance
> so that I can track the original values and modified values, which is
> perfectly reasonable. If I used a class in this situation, I would
> have to refer to all 100 members to make a copy of the object.
No, you would have to implement ICloneable on the class and write a
Clone method to copy each of the 100 members. That's what Clone is for.
This example is even worse than the original situation. Do you not
realize that a struct with 100 members would be _copied_ on ever method
call, every variable assignment, every time it were put into an
aggregate structure (such as an ArrayList) and every time that it were
removed? On top of the massive inefficiency that that entails, the
amount of pain and suffering you would endure trying to wrangle a
100-member, mutable type with value semantics would... well, it would
convince you never to try that again. :-)
> I think is is not appropriate to make general statements like "don't
> use a struct in this situation, use a class". Leave that up to the
> individual to decide based upon what they're trying to solve.
Well, then we disagree. I'm trying to give you the benefit of my
experience here. If you want to forge ahead with structs, then go for
it. I predict that sooner or later you'll decide that it was a bad
design. A lot of ex-C / C++ programmers come through this group and
microsoft.public.dotnet.languages.csharp thinking that "struct" in .NET
is some sort of "class-lite" and use it as such. It's not. It has very
specific uses. I don't think that you fit into this group, because you
understand value semantics, but there's a lot of misuse of struct out
there. I'm just trying to save you days or weeks of heading down the
wrong path.
I have fairly simple rules for when to use structs and when to use
classes. Microsoft has some rules too, but I find that they fall a bit
short of these ones.
1. Anything that has _identity_ should be a class. Always. If it has an
ID then it should be a reference type, a class: customers, suppliers,
branch offices, employees, invoices, purchase orders, ... they all have
unique identifiers, so they should be classes.
There are many reasons for this rule. Things with identifiers tend to
fit nicely into an inheritance hierarchy, and it's handy to be able to
take advantage of polymorphism, interfaces, and that sort of thing;
structs cannot participate in inheritance, whereas classes can. Things
with identifiers tend to be mutable: you want to be able to change a
customer's name, for example, or his address. This brings us to point
#2.
2. Anything that is _mutable_, that is, has properties with "set"
methods, should almost certainly be a class. There are exceptions (see
below), but in 99% of the cases this is true.
Mutable value types are trouble. They're just plain confusing, even to
people who clearly understand the difference between reference and
value types. One of the FAQs in the C# newsgroups has to be why this
doesn't work:
ArrayList a = new ArrayList();
a.Add(new Point(5,5));
((Point)a[0]).X = 6;
Console.WriteLine((Point)a[0]);
Why does this print 5,5 and not 6,5? It's because of boxing and value
semantics. Now, do this with a Customer that has 25 fields, all
mutable, and watch maintenance programmers get very, very confused
about why their changes simply have no effect. This was the nut of your
question. This is why I told you to use a class: you're creating a
mutable struct for no apparent good reason. Mutable structs are
difficult to work with, so you'd better get some serious benefits from
using struct in order to make that pain worthwhile.
Perhaps this is a "trust me" item. I started out using structs for
things like this, exactly like in your original example. It started out
well enough, but then after a couple of days I found myself scratching
my head. After a week I realized that I had made the wrong decision and
I backtracked and used classes. Trust me: I'm pretty stubborn. I tried
"everything" in order to get the struct thing to work until I finally
figured out that I was trying to use a screwdriver to pound a nail into
the wall. I decided to use a hammer instead (classes).
Structs act like values: ints, longs, doubles, dates, times.
Programmers just do _not_ expect a Customer to act like an int and be
copied all over the place. They _do_ expect a Customer to act like a
class, a reference type.
3. DO use structs if you are writing something that should act like a
value. For example, if it participates in mathematical expressions.
I'll give you some examples of structs that I have in my system, and
you'll quickly get the idea:
Fraction - Holds a whole part, numerator, and denominator.
Measure - Holds a decimal quantity and a unit of measure like feet,
pounds, cubic centimeters, etc. These can also participate in
mathematical expressions: 1 foot plus 6 inches = 1.5 feet, etc.
MeasureFraction - Same as measure, but holds a fraction and a unit of
measure
MonetaryAmount - Holds a decimal quantity and a currency.
All of these things _should_ act like native values: when you assign
them, you expect to get a copy, viz:
Fraction a = new Fraction(0, 1, 2); // 1/2
Fraction b = a;
Fraction b = b + a;
You expect to get 1 in b, and still have 1/2 in a. Of course, that
makes sense, since Fraction is immutable: you can't change anything
about it... every operation returns another fraction that you can store
somewhere, but you can't change a Fraction once it's created (see point
#2).
4. Structs should be _small_. Even Microsoft agrees on this one: they
say 16 bytes or less. Why? Because if it's a struct then it's going to
be copied. A LOT. It's going to be copied on the stack, copied inside
classes, copied, copied, copied. If you have a 1K struct then
pretty-much any time you do _anything_ with that struct you are copying
1024 bytes. Large structs will kill the performance of your program.
5. Finally the caveat to point #2: sometimes you make mutable structs.
Microsoft did it with Point and Rectangle, for example. Search
microsoft.public.dotnet.languages.csharp for "Point" and "struct" and
you'll turn up dozens of discussions started by confused programmers
who couldn't figure out why Point (the most popular of the mutable
structs) acts so weird. Moral: this is not to be done lightly. It
confuses people. It makes your code hard to maintain.
So why did MS do it? Because inside their managed graphics code they
were doing lots of mathematics involving Points and Rectangles.
Mathematics creates intermediate results, which would have meant, if
these were classes, hundreds or thousands of Points and Rectangles that
were only needed for a fraction of a second that had to be instantiated
on the heap and then eventually garbage collected. The number of Points
and Rectangles involved was so large (hundreds or thousands per second,
I would guess) that putting them on a heap incurred a significant
performance penalty. So, they made them mutable structs.
Why mutable? Well, they wanted you to be able to use syntax like:
myForm.Location.X = 500;
instead of forcing you to say:
myForm.Location = new Point(500, myForm.Location.Y);
Me, I would have made Points and Rectangles immutable and forced people
to do the latter, but that's just me, probably because of all of the
confusion I've seen in the newsgroups. :-)
This last rule is hardly ever needed in "normal" code. I'm almost never
doing anything so intensive with a class, producing so many
intermediate results, that making it a mutable struct would be an
attractive option. If you're doing ray tracing or analyzing massive
amounts (gigabytes) of data then you may make different decisions.
So, to sum up, there _are_ good uses for structs. When you _really_
need one, you find out how wonderful it is to be able to create your
own almost-native type that can stand shoulder-to-shoulder with int,
double, and DateTime. That said, most people trying to use structs
press them into service for entirely the wrong reasons: they use a
struct where a class would have been a much better choice. What you
were trying to do in your example indicated that kind of choice to me:
use a class.
Bruce Wood
> > Obviously the compiler is not making a copy, which seems to be inconsistent behavior.
>
> No, no. The compiler _is_ making a copy, which is precisely the
> problem, viz:
>
> 1. Make a copy of the value in the ArrayList.
> 2. Change some property of the copy.
> 3. Throw the copy away.
> 4. Repeat for each item in array list.
>
> It's a completely useless operation.
Ahh... I see. The compiler complains whether you do something
legitimate with the altered struct or not. A better example, then,
would have been:
foreach(Address objAdr in arlAddress)
{
objAdr.city = "cleveland";
Console.WriteLine("{0}", objAdr.ToString());
}
... or something like that, which does something useful with the
modified struct before tossing it away. Even so, the compiler
complains, which makes sense when you think about it. The _value_
returned on each foreach is not changeable. You can't do this, either,
by the way:
foreach (string s in arlAddress)
{
s = null;
Console.WriteLine("{0}", s);
}
The compiler responds with "error CS1604: Cannot assign to 's' because
it is read-only". Just the same as if you said,
foreach(Address objAdr in arlAddress)
{
objAdr = new Address(...);
Console.WriteLine("{0}", objAdr.ToString());
}
So, you're not allowed to modify the value of a foreach indexer, which
seems a sensible restriction. However, you were modifying the property
of a "struct" within the foreach loop, but remember that a "struct" is
a _value type_, so modifying one of its properties means, logically,
modifying the value. So, for a struct, trying to modify any property
would constitute the same kind of property as trying to assign it a new
value. So, the compiler doesn't allow it. Apparently it does this by
saying that the property of a read-only object or value can only ever
be an r-value, never an l-value. Thus the error message.
You should still be using a class, though, not a struct. ;-)