Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does resharper suggest "wrap variable in array" for access to modified closure warnings?

Tags:

Given the following (heavily edited, pseudo-)code:

int count = 0;
thing.Stub(m => m.AddBlah()).WhenCalled(o => count++);
thing.Stub(m => m.RemoveBlah()).WhenCalled(o => count--);

DoStuff(thing);

Assert.AreEqual(1, count);

ReSharper provides a warning on count - "Access to modified closure". I understand why I'm getting this warning (the count variable is being modified in two different lambdas, and is likely to have undesirable semantics), but I don't understand ReSharper's advice: "Wrap local variable in array". If I let ReSharper do this, I get:

int count[] = { 0 };
thing.Stub(m => m.AddBlah()).WhenCalled(o => count[0]++);
thing.Stub(m => m.RemoveBlah()).WhenCalled(o => count[0]--);

DoStuff(thing);

Assert.AreEqual(1, count[0]);

And no warning.

Why is using an array safe?

like image 435
citizenmatt Avatar asked Jun 24 '11 11:06

citizenmatt


1 Answers

I noticed this same thing in ReSharper myself, and was also left wondering why it doesn't warn when the value is wrapped in an array. The other answer here is unfortunately wrong, and seems to misunderstand how closures are implemented, so thought I would attempt explain (what I think is) the rationale behind this refactoring.

As you've seen, the result is the same whether array-wrapped or not, so the refactoring doesn't really "fix" anything and the same issues that can be encountered when accessing an ordinary modified closure exist after applying the change. However, after the change since the count array itself is not being modified (only its contents), the "Access to modified closure" warning is no longer relevant.

The change doesn't really make the problem any more obvious in (at least in my opinion), so it would seem that this suggestion is essentially telling ReSharper to ignore the issue, without having to resort to the rather messy // ReSharper disable AccessToModifiedClosure mechanism to suppress the error.

like image 171
Iridium Avatar answered Sep 30 '22 18:09

Iridium