#Google Analytic Tracker

Pages

Mar 20, 2007

Some Danger of Refactoring using ReSharper

No doubt, ReSharper (2.5 specific) had helpped me a lot when coding in C#. Particularly I like its Reformat code feature which it removes redundancy in the code.

For example

private void DoSomething(object sender, EventArgs e)
{
   DoStuff(e);
}


private DoStuff(EventArgs e)
{
...
}

Resharper will suggest to remove the parameters "sender"

However, what if your function looks like the following:

private void DoSomething(object sender, EventArgs e)
{
    if (this.m_oLogGridView.InvokeRequired)
{
this.Invoke(new EventHandler(DoSomething), new object[] {this, new object()});
}
else
{
DoStuff(e);
    }
}

Resharper will suggest you to remove the parameter "sender". However, if you remove the sender parameter without fixing "this.Invoke(method target, parameters)" function, you will run into runtime error. That's because the number of parameter we are passing using the this.Invoke() doesn't match the method parameter.

So, be careful when use refactoring.

4 comments:

Ilya Ryzhenkov said...

This is a bug. I've created request in our JIRA database, you can observe its status here: http://jetbrains.net/jira/browse/RSRP-38316

Ilya Ryzhenkov said...

However, there will be compile time error, as method signature will not match EventHandler.

Also, I think it is not very good idea to check InvokeRequired on one Control and issue Invoke on another. In theory they could be created on different threads :)

Ilya Ryzhenkov said...

And one more problem with your code - new object() you pass in parameters cannot be casted to EventArgs. May be this is your runtime error? :)

Dicky said...

Wow, I am surprise I got some feedback. Thank for the comments. I am a newbie, so please do point my mistake if I am wrong.

I agree it is not a good idea to use check InvokeRequired. I check the InvokeReRequired because I am trying to avoid "cross-thread operation exception" where 2 threads tries to access the same control. This is not a preferred way to fix the problem according an article I found. It suggested to use BackgroundWorker, but currently I am not able to get it to work.

In addition, I haven't test the sample code that I put on the blog, which I should, but haven't get the time.

However, the point that I was trying to make is exactly that I could get an runtime error if I simply remove the unused parameters. It is more a prediction because like I said, I haven't test the code.

More testing tonight, thanks :)