Invoke delegates without params but using local params c#

Question

I find myself doing the following a lot, and i don't know if there is any side effects or not but consider the following in a WinForms C# app. (please excuse any errors as i am typing the code in, not copy pasting anything)

int a = 1;
int b = 2;
int c = 3;
this.Invoke((MethodInvoker)delegate()
{
    int lol = a + b + c;
});

Is there anything wrong with that? Or should i be doing the long way >_<

int a = 1;
int b = 2;
int c = 3;
TrippleIntDelegate ffs = new TrippleIntDelegate(delegate(int a_, int b_, int c_)
{
   int lol = a_ + b_ + c_;
});

this.Invoke(ffs);

The difference being the parameters are passed in instead of using the local variables, some pretty sweet .net magic. I think i looked at reflector on it once and it created an entirely new class to hold those variables.

So does it matter? Can i be lazy?

Edit: Note, do not care about the return value obviously. Otherwise i'd have to use my own typed delegate, albeit i could still use the local variables without passing it in!

Answer 1

The way you use it, it doesn't really make a difference. However, in the first case, your anonymous method is capturing the variables, which can have pretty big side effects if you don't know what your doing. For instance :

// No capture :
int a = 1;
Action<int> action = delegate(int a)
{
    a = 42; // method parameter a
});
action(a);
Console.WriteLine(a); // 1

// Capture of local variable a :
int a = 1;
Action action = delegate()
{
    a = 42; // captured local variable a
};
action();
Console.WriteLine(a); // 42

Answer 2

From a style perspective I'd choose the paramater passing variant. It's expresses the intent much easier to pass args instad of take ambients of any sort (and also makes it easier to test). I mean, you could do this:

public void Int32 Add()
{
    return this.Number1 + this.Number2
}

but it's neither testable or clear. The sig taking params is much clearer to others what the method is doing... it's adding two numbers: not an arbatrary set of numbers or whatever.

I regularly do this with parms like collections which are used via ref anyway and don't need to be explicitlly 'returned':

public List<string> AddNames(List<String> names)
{
    names.Add("kevin");
    return names;
}

Even though the names collection is passed by ref and thus does not need to be explicitly returned, it is to me much clearer that the method takes the list and adds to it, then returns it back. In this case, there is no technical reason to write the sig this way, but, to me, good reasons as far as clarity and therefore maintainablity are concerned.

Answer 3

There's nothing wrong with passing in local variables as long as you understand that you're getting deferred execution. If you write this:

int a = 1;
int b = 2;
int c = 3;
Action action = () => Console.WriteLine(a + b + c);
c = 10;
action();  // Or Invoke(action), etc.

The output of this will be 13, not 6. I suppose this would be the counterpart to what Thomas said; if you read locals in a delegate, it will use whatever values the variables hold when the action is actually executed, not when it is declared. This can produce some interesting results if the variables hold reference types and you invoke the delegate asynchronously.

Other than that, there are lots of good reasons to pass local variables into a delegate; among other things, it can be used to simplify threading code. It's perfectly fine to do as long as you don't get sloppy with it.

Answer 4

Well, all of the other answers seem to ignore the multi-threading context and the issues that arise in that case. If you are indeed using this from WinForms, your first example could throw exceptions. Depending on the actual data you are trying to reference from your delegate, the thread that code is actually invoked on may or may not have the right to access the data you close around.

On the other hand, your second example actually passes the data via parameters. That allows the Invoke method to properly marshal data across thread boundaries and avoid those nasty threading issues. If you are calling Invoke from, say, a background worker, then then you should use something like your second example (although I would opt to use the Action<T, ...> and Func<T, ...> delegates whenever possible rather than creating new ones).