How to deal with the "Access to modified closure" warning

Question

Given the following code snippet:

string body = "blabla";

/* some processing */

itinerary.Propositions.Each(prop =>
{
    /* some processing */
    body += "looking good";

    /* some more processing */
});

Resharper is throwing the "Access on a local closure" error. Now, I've read a lot about this, but so far not a way to avoid the error in situations like this.

So... how can I still use the variable body without having the warning?

Is this the exact error message? I find it strange for resharper to use `closure` in the context of lambdas — Matan Shahar, Feb 26 '19 at 17:29
@MatanShahar, it is not strange that resharper complains. Resharper doesn't know if the code inside `itinerary.Propositions.Each` that will invoke the delegate is being executed synchronously or asynchronously in some way... — , Feb 26 '19 at 17:31
Well, I should add I'm only getting this warning since installing a plugin which "completes" some of the testing by Resharper. I understand the theory, just wonder how I can avoid getting the warning. — Fysicus, Feb 26 '19 at 17:32
@elgonzo I'm not saying that the error itself is unlikely, the wording seems off so I ask for the error verbatim. — Matan Shahar, Feb 26 '19 at 17:33
@MatanShahar, the wording is not off. Some background: https://stackoverflow.com/questions/9591476/are-lambda-expressions-in-c-sharp-closures — , Feb 26 '19 at 17:34

D Stanley · Answer 1 · 2019-02-26T17:36:29.327

0

Well, you are modifying a variable within a lambda that is declared outside the lambda, which is a warning. So either ignore the warning or don't modify it within a lambda. If you understand why it's a warning then you'll be in a better position to know if you should ignore the warning or not.

how can I still use the variable body without having the warning?

Just use foreach?

foreach (var prop in itinerary.Propositions)
{
    /* some processing */
    body += "looking good";

    /* some more processing */
}

edited Feb 26 '19 at 17:36

answered Feb 26 '19 at 17:30

D Stanley

149,601
11
178
240

Is `body` used somewhere else within another lambda? With what you've posed there _are_ no closures. – D Stanley Feb 26 '19 at 17:33
Yes, I've already thought of that. But it would mean a "drastic" change in the code base and it would be probably not be approved. As ridiculous as it may sound... – Fysicus Feb 26 '19 at 17:33
You need to provide more context so we can understand why this is a "drastic" change and provide a different solution. – D Stanley Feb 26 '19 at 17:37
Ow well... the code reviewer here doesn't like it when you change constructs with no good reason. I don't have to change anything in this part of the code, so a small change would be accepted, but changing the flow by introducing a new construct is not. – Fysicus Feb 26 '19 at 17:42

score 0 · Answer 2 · answered Feb 26 '19 at 17:38

If you are building a string, a loop with += is not optimal. It creates a lot of new immutable instances to be garbage collected, and can be rewritten in a more beautiful way.

Maybe:

string body = "blabla" + string.Concat(itinerary.Propositions
  .Select(prop => { /* some processing */ return "looking good"; })
  );

If delimiters are desired, use string.Join instead of string.Concat.

score 0 · Answer 3 · answered Feb 26 '19 at 17:51

0

If you want just disable warning:

// ReSharper disable once AccessToModifiedClosure
body += "looking good";

or Alt+Enter on "body", then

answered Feb 26 '19 at 17:51

Timur Lemeshko

2,747
5
27
39

How to deal with the "Access to modified closure" warning

3 Answers3