I have the following code example taken from the code of a Form
:
protected void SomeMethod()
{
SomeOtherMethod(this.OnPaint);
}
private void SomeOtherMethod(Action<PaintEventArgs> onPaint)
{
onPaint += MyPaint;
}
protected void MyPaint(PaintEventArgs e)
{
// paint some stuff
}
The second method (SomeOtherMethod
) has resharper complaining at me. It 开发者_运维技巧says of onPaint
that "Value assigned is not used in any execution path".
To my mind it was used because I added a method to the list of methods called when a paint was done.
But usually when resharper tells me something like this it is because I am not understanding some part of C#. Like maybe when the param goes out of goes out of scope the item I added to the list gets removed (or something like that).
I thought I would ask here to see if any one knows what resharper is trying to tell me.
(Side Note: I usually just override OnPaint. But I am trying to get OnPaint to call a method in another class. I don't want to expose that method publicly so I thought I would pass in the OnPaint group and add to it.)
The warning is correct. Consider the following:
int X;
int Y;
void SomeMethod()
{
SomeOtherMethod(this.X);
}
void SomeOtherMethod(int x)
{
x += this.Y;
}
Here the code modifies formal parameter x, and then never uses the modified x. This does not modify "this.X"
You've done the same thing with the delegate. You modify the formal parameter and then never use the result; the original "OnPaint" is unchanged, just as "X" is unchanged in my example.
Remember, just because a delegate is a reference type does not mean that you're passing around a reference to a variable when you pass around an instance. You're passing a reference to an instance, not a reference to the storage location of that instance.
Delegates are immutable. You can't change them. They're a bit like strings in that respect. Imagine your method was:
private void SomeOtherMethod(string x)
{
x += "hello";
}
Again, that would be a pointless method. The original string wouldn't have changed - you'd just have changed the value of the local variable (the parameter) to refer to a different string. The exact same thing happens in your delegate case.
Either you need to pass the variable by reference or you need to change your whole design.
See my article on delegates and events for more about how delegate combining works.
Delegates are immutable, so combining creates a copy. When you call:
private void SomeOtherMethod(Action<PaintEventArgs> onPaint)
You're actually creating a modified copy of the original Action<PaintEventArgs>
.
That being said, I would personally avoid trying to work this way, unless there is a very compelling reason to do so.
Personally, I would consider making an interface which exposes an OnPaint
event, and passing the interface into this method. You could then subscribe to the event. This would have the same effect you're trying to achieve, but be much more clear.
In this case, I would just have your other class subscribe to the Paint event on the Control directly.
Does it work at all? I don't expect that MyPaint is ever called. That would be a clue.
It seems to me that onPaint += MyPaint;
would have no effect outside this method. onPaint
is a parameter (local variable), and the changes are lost when the method exits. And that is why you get the warning.
To see why, you would have needed something like a ref
parameter, but you can't call that with an event (this.Onpaint):
// not applicable
private void SomeOtherMethod(ref Action<PaintEventArgs> onPaint)
{
onPaint += MyPaint;
}
After doing some experiments, I think the reason is that when you do a +=
on an Action
, you are not modifying the initial value, but rather the local variable itself. For example:
void Main()
{
Action<int> doSomething = OnClick;
doSomething += i=> Console.WriteLine("test");
OnClick(1);
}
private void OnClick(int i)
{
Console.WriteLine("clicked");
}
... simply yields "clicked".
So the MyPaint method in your example is not being modified, only the onPaint variable. Since you don't do anything with that variable after the +=
, there was no point doing the +=
in the first place.
Edit
Porges points out that since OnClick is a method (not a delegate), this example isn't quite accurate. Here's a better one:
Action<int> doSomething = i => Console.WriteLine("test");
var doSomething2 = doSomething;
doSomething2 += i => Console.WriteLine("test2");
doSomething(1);
Output: test
精彩评论