C#: Cannot assign to foreach iteration variable

Question

I'm trying to condense this by wrapping it in a loop:

       if (pos.X + pixelRadius < 0) {
            pos.X = bounds.Width - pixelRadius;
        } else if (pos.X + pixelRadius > bounds.Width) {
            pos.X = pixelRadius;
        }

        if (pos.Y + pixelRadius < 0) {
            pos.Y = bounds.Heigh - pixelRadius;
        } else if (pos.Y + pixelRadius > bounds.Height) {
            pos.Y = pixelRadius;
        }

My first instinct was to do something like this:

        foreach (float coord in new float[] { pos.X, pos.Y }) {
            float upperBound = (coord == pos.X ? bounds.Width : bounds.Height);
            if (coord + pixelRadius < 0) {
                coord = upperBound - pixelRadius;
            } else if (coord + pixelRadius > upperBound) {
                coord = pixelRadius;
            }
        }

But of course then I get the error message:

Cannot assign to 'coord' because it is a 'foreach iteration variable'

Is there any way I can wrap this code in a loop? Or maybe it's not worth the effort, and it's more readable to leave in the first form.

For those who are curious: yes, this is implementing wrap-around.

Answer 1

Honestly, I think the first version is more readable. The second version batches stuff up in a loop and then ... tests to see which loop element it is??? Smells like for...case antipattern to me.

Answer 2

Even if you could change coord, since it's a float you'd actually just be modifying a copy and not the value in pos. Your best bet is to create a new method.

Answer 3

I would leave it in it's original form. When you're in the loop like above, coord is a COPY of the pos.X or pos.Y variable since they're Value Types. This means even if you could update coord...you wouldn't be updating pos.X or pos.Y, but a copy of them local to the loop.

Answer 4

I think this is trying to do modulo math, so this is really what you want

pos.X %= bounds.Width;
pos.Y %= bounds.Height;

It doesn't give the exact same behavior as the above code, but it could if you just adjust the bounds and apply a bias to the point before doing the modulo.

or this if you need to have a bias

pos.X = ((pos.X - pixelRadius) % bounds.Width) + pixelRadius;
pos.Y = ((pos.Y - pixelRadius) % bounds.Height) + pixelRadius;

Modulo math is a better way to do wrap-around. It's clearer, and it has no branching.

Answer 5

The loop is a bad idea.

The code You have now, looks buggy, but it's structure is better, than some unnatural use of a loop.

If I understand what You are trying to do, I think You should have something like this

(I'm focusing on coordinate X. In case of coordinate Y it's analogous)

pos.X += pixelRadius;
if(pos.X < 0) {
    pos.X += bounds.Width;
} else if (pos.X > bounds.Width) {
    pos.X -= bounds.Width;           
}

This will work if |pixelRadius| < bounds.With and I'm assuming the numbers are floats, as the example with a loop suggests.

Modulus operator makes things a bit easier especially if |pixelRadius| > bounds.With, but be careful in case of negative numbers. You have to know exactly what Your language implementation is going to do with them

Using the modulus operator I would do it like this

pos.X = (pos.X + pixelRadius) % bounds.Width;
if(pos.X < 0) pos.X += bounds.Width;