Welcome to MSDN Blogs Sign in | Join | Help

JoeN's Blog

XNA Shaman
Cascading changes from a method rename - what do you think?

I'm working on an issue with the C# refactoring and wanted to get your opinion. One of the things we are delivering in VS 2005 is the ability to reliably rename methods, types etc.

We have a current issue where you can be in some code and you want to rename a method. The method is actually part of an interface definition (let's ignore the fact that we currently don't have a way to indicate that the method is part of an interface).

There are several ways to interpret renaming this method. Our current implementation does this:

  1. Changes the name of the method in the class that you are editing (this is where you kicked the rename off)
  2. We detect that the method is part of an interface declaration so we change the interface to match the new name.
  3. We then detect all other implementations of the interface and change the methods in those types to match the new name.

I'm a little concerned that this is a little too “easy” and may cause more changes than a developer was expecting. We have a number of ways to fix this (inform the user that this is happening, come up with a way of selectively choosing which replacements to make via the preview dialog) but I'm inclined to perhaps simply restrict this so that you can only rename an interface methods from the interface itself so it's a much more deliberate action.

We are still tossing this around but I'd love to get your feedback on what you would expect.

Posted: Tuesday, June 08, 2004 9:31 PM by JoeN
Filed under:

Comments

Steve Maine said:

The default behavior sounds kind of far-reaching to me. Renaming a class method should not affect the names of methods in other classes.

I do like the idea of the "implementation method rename", but I'd like to have control over it. Preferably without a preview dialog, too. What about the following:

If the cursor is currently positioned inside of a class definition, the method rename only affects the current class. If that method happens to be an interface implementation, the class will no longer fully implement the interface after the refactoring. The compiler will complain if I don't replace the interface implementation, so if I've screwed up I'll know as soon as I recompile.

However, if I navigate to an interface declaration and rename a method defined on that interface, that change should propagate to all classes in the current solution that implement that interface. All classes that fully implemented the interface before the rename will still implement the interface after the refactoring.

Changing a class shouldn't automatically change the interface it implements, but changing an interface should change all its implementations.
# June 8, 2004 10:28 PM

JimG said:

I would certianly prefer that changes to the interface only occur if you made the change to the interface itself (or at least a very strong warning). Of course the same question could be asked about changes to overridden methods although it's easier to tell that a method is overridden than it is to tell it is implementing an interface.
# June 8, 2004 10:38 PM

Robert Jeppesen said:


I agree. Do NOT make editing the method name so that it renames the method name in the interface.
Altering an interface ALWAYS need to be a conscious decision. If it is, the developer is likely to rename the interface first and not the implementations of the interface.
So when renaming in the interface, it should affect all classes implementing that interface (in the current solution), but never the other way around.
That would make it way too easy to break stuff.

# June 8, 2004 11:14 PM

Geoff Appleby said:

I definately agree with the idea of only renaming all interface implementations if and only if you do the rename on the interface.

One question this leads me to:
If you do the method rename via refactoring on and interface, it would be nice if it propogated to all other classes implementing it in the current solution. But what if you change the name by hand in the text editor? In this case, has teh 'refactoring rename' been bypassed?
# June 8, 2004 11:35 PM

David James said:

On a related issue, will VS2005 have any way to do a find-and-replace rename of several methods at once? E.g. given a class and its unit tests, both in the same source file:

class X
{
public void AddClient();
public void DeleteClient();
}

class XTest : TestCase
{
public void TestAddClient();
public void TestDeleteClient();
}

It would be nice to say "rename Client to Customer", and have it change all the method names appropriately.

If I ever wanted to rename a single method, I would always want to rename its unit test method(s) too.
# June 9, 2004 12:12 AM

Eric Quist said:

I totally agree with all the previous statements.

I also got an idea about what could happen if you choose to rename a method that is part of an interface implementation. One solution is to let the rename result in a compile error. But a second solution could be to add a new method that implements the interface and does so by calling the renamed method?
# June 9, 2004 1:24 AM

Paul Blamire said:

I would definitely prefer to rename the method on the interface explicitly. I guess it's just a feeling of control thing....
# June 9, 2004 1:45 AM

Mark Allan said:

Personally, if I renamed an interface method from within a class, I would probably expect only a class-level rename - if I was intending to change the interface definition then that's where I'd be doing it from. But the pragmatist in me says that you're not really going to be able to guess what the developer intends, so some sort of dialog is going to be required, either just warning that this is an interface method and that you should be modifying the interface if that's what you want (probaby best - easy to implement and would allow you to turn off the warning), or offering the choice of a class or interface level rename. Offering a list of potential renames might work, but it would need a good UI!
# June 9, 2004 1:48 AM

Jonny G said:

Hi !
I assisted to your TechEd presentation on IDE Enhancements. It was greeeat !

I think it could be convenient to ask the developer from the refactor method dialog box, what he exactly want to do. Please don't lock everyone into the same behavior. There is cases where it could be convenient to stay in the current class code and apply the changes in the interface, in the class itself, and/or all overriden versions of the method. It will be great if we have control on this.
# June 9, 2004 5:18 AM

Omer van Kloeten said:

I agree with the overwhelming wave of "Rename the interface only if explicitly renaming it" comments and would also like to suggest that you would be able to create a private implementation that would call the new method.
Example:

public interface I
{
int Something(int i);
}

public class A : I
{
public int Something(int i) {}
}

Renaming Something to Something1 would cause:

public interface I
{
int Something(int i);
}

public class A : I
{
// TODO: Change me?
int I.Something(int i) { return Something1(i); }
public int Something1(int i) {}
}

This way, you could even recompile the program with no changes, since it still implements the interface.
# June 9, 2004 8:37 AM

Chris Lucas said:

At least in my code interfaces enforce contracts between components. I'm not sure I would want that contract to change so innocuously; but Omer's suggestion is a great one, since often the same method is used to fulfill the contract and provide functionality used elsewhere within the class - allowing you to change the latter w/o the former is a great idea.
# June 9, 2004 12:29 PM

Matthew Humphrey said:

For what it's worth, when you do this in IntelliJ IDEA, you get a dialog box that asks:
"Method xxxx() of class yyyy implements method of interface zzzz. Do you want to rename the method from interface?" If you say yes it does the refactoring in all the classes implementing the interface. Of course, the default behavior in IDEA is to bring up a window that shows you each refactoring that will occur, so you still get one more chance to reject the change.

These guys are on their 3rd iteration of this refactoring stuff, and they really have some good ideas. I hope you guys at least look at it.
# June 9, 2004 6:55 PM

Mike Mayer, C# MVP said:

My first thought was that renaming everything seemed like a bad idea. It won't work at all if the interface is defined in another assembly.

But a worse solution is to just renames a method at the class level (and not the interface + all classes using the interface). Then you are suddenly changing the way a program works (and quite probably breaking it all together).

My opinion (like many above) is to only allow method renames from the interface. If a user tries it from the class, and you can easily find the interface (in the same assembly) give them a dialog with a link "Go to interface definition" that will open up the interface file, position cursor on the method defintion, then the user can Refactor>Rename again. Probably the most straight forward way to implement it.

# June 17, 2004 3:49 PM

Justin said:

Changes brought about by refactoring using the rename method should always only atttempt to affect downstream occurances. Upstream changes should require the rename action be [explicitly] initiated at that upstream location.

Methods or instance variables that override inherited or interface implementation members should require explicit user-interaction. During this interaction, the user should be presented with at least three options:

1. Apply the proposed change relative to the current item of focus. The proposed change would include:

a) Perform the rename allowing the interface or parent class relationship/contract to be broken.
b) If it is a method (formal property and maybe even an event), create a new method (retaining the old method signature) that encapsulates the change by calling the old unchanged method. Possibly apply to the unchanged method a code attribute that indicates deprecation. An exception to this would be explicit implementation.
2. Skip the current occurance.
3. Apply to all currently unreviewed occurances (eg fast-forward the application of the change to the remaining downstream occurances).

All three strategies for dealing with inheritence and interface members, along with with basic downstream rename should be 'undoable' and 'redoable'.

Currently, in the VC# Express Edition, a rename action on a member that implements an interface member will prompt to make the change, but warns that the member was defined in a referenced assembly and if continuing with the change, it will cause a compile error. In this warning, you are presented with a 'Yes/No' dialog, to continue with the change. If 'No' is selected the change is not made (as expected), but you are then presented with an 'Ok' dialog indicating the 'Rename failed'. If no changes were requested, the 'Ok' box should not be displayed in this case (for future iterations of Expres or the full version of VS.Net Whidbey). The only time that 'Ok' box should be displayed (if ever) should be when the 'Yes' option was originally selected to continue with the rename against an inherited/interface implementation member and no changes resulted (for some unknown reason).
# July 22, 2004 3:45 PM
Anonymous comments are disabled
Page view tracker