Are Refactorings safe?

Published 26 February 04 10:04 AM

The C# refactorings we're building in Whidbey are designed to be reliable. 

Not all of our customers use TDD, but many will use the refactoring tools we build.  If the tools don't work reliably, users will stop using them.

The big exception was unbuildable code.  We can't really guarantee preserved semantics of illegal code, since the semantics are undefined!

If you're doing TDD, this exception matters less, as you keep your code buildable pretty much all the time.  It's a prerequisite to running the tests, right?

There are some other subtle exceptions:

  1. Reordering the parameters to a method can have an unforeseen effect if the parameters you pass have side effects.  The side effects now happen in a different order.
  2. Removing a parameter to a method will change behavior if you pass a parameter with a side effect.

Both of these are difficult to guard against.  What should we do?  Remove the refactorings because they're unsafe?  Add cumbersome warning messages on the UI?

However, we're definitely working hard to make Rename and ExtractMethod very reliable.  They're the most important Refactorings we can provide, and we want you to use them a lot.

Filed under:

Comments

# Darrell said on February 26, 2004 10:46 AM:
I would prefer warning messages to removing the functionality, especially with a "do not show this message again" option. :) Why limit advanced users in catering to the less-advanced users?
# Olle de Zwart said on February 26, 2004 11:00 AM:
I agree 100% with Darrell
# Omer van Kloeten said on February 26, 2004 11:17 AM:
+1 for Darrell.

You should do the same when a user changes from code view to design view and tries to change something in the Windows Forms designer.
If you try and do that in VS.NET 2002/3, either some or almost all of your work is lost and is unrecoverable.
That just sucks.
# Jay Bazuzi [MS] said on February 26, 2004 11:34 AM:
You mean when you edit in the area that says "Do not edit"? *wink*
# Steve Hall said on February 26, 2004 11:36 AM:
What about inserting comments that are easy to find, like the // TODO: comments generated by C++ wizards? Something like: // TODO: Inspect the following refactored statement for this potential problem: xxx xxx.

And when these potential problems could occur, I would even want the original statement left as a comment after the above // TODO, to serve as a reference to the old code.

Also, adding project setting options to turn on/off prompts and another one to turn on/off comments would be good to have to control this behavior.

Also, there should probably be at least one project setting checkbox to turn on/off all unsafe refactorings. Ideally, you could have separate checkboxes for each type of refactoring and warn the user when exiting out of the project settings property page that they've chosen some unsafe refactoring methods.

If you choose to implement only prompts, then maybe you could have buttons like: "Skip", "Proceed - No Comment", and "Proceed - Comment" to allow for warning comments to be left behind.

In any case, please don't remove unsafe refactorings, as that dilutes the entire idea of refactoring (taking some risk to improve the code). Instead, offer some assistance if it can't be automatic (safe).
# Ferris Beuller said on February 26, 2004 12:16 PM:
WInform designer isnt reliable, resx files are a joke that cant refactor too well . I stopped supporting .resx and winform designers.

# Ferris Beuller said on February 26, 2004 12:16 PM:
Yeah and ANY code thats in a project, I will edit if I damn well see fit. If your tools cant handle it I stop using it.

# Michael Teper said on February 26, 2004 12:29 PM:
+1 for Darrell
# Frans Bouma said on February 26, 2004 12:47 PM:
I'm perhaps missing something but what's TDD?

I agree with Darrell btw.
# senkwe said on February 26, 2004 12:50 PM:
TDD == Test Driven Development. If that was a serious question, it warms my heart that you can still create excellent software (LLBLGenPro) without necessarily requiring TDD (Of course you might have done it that way, just under a diff name)
# Omer van Kloeten said on February 26, 2004 1:28 PM:
Jay- Which "Do not edit" area are you talking about? I have seen nothing like that...
# Naginata said on February 26, 2004 1:35 PM:
Wow, I'm glad you posted your link Frans, if I ever clear all the red flags off my desk I'm going to have to take a look at the LLBLGenPro demo :)

My feeling on refactorings is... I hate getting warnings and notifactions popping up at me, because 90% of the time, I already know that what I did was potentially dumb, and I want to do it anyway, and the 10% that I SHOULD read the warning, I never do because I'm trained to just click ok/cancel/ignore/whatever.

I think that leaving comments around can be one way to do it, especially on unbuilt/dirty (as in edited) code. I also think that the new "color coding" features going in to whidbey could be used... maybe have a special color for code that's been changed by the refactoring tool since the last build or something?
# Robert Watkins said on February 27, 2004 4:07 AM:
Jay,

One option to consider with the refactoring is a preview mode, where the user can inspect the (possibly dangerous) refactoring and decide for themselves.
# Dan Fernandez's Blog said on April 5, 2004 4:59 PM:
# ABS said on August 27, 2004 8:56 AM:
New Comments to this post are disabled

This Blog

Syndication

Page view tracker