Reorder parameters


One of the refactorings we’re including in C# 2.0 is “reorder parameters.” Strangely enough, it doesn’t do what you think it might do. Wait… nevermind. It does exactly what you think it might do, it allows you to reorder the parameters in a signature. Any that calls that will then have their arguments reordered appropriately.

This is, of course, something you could do manually (all refactorings are), however it’s potentially painful and possibly error prone. It’s painful when you reorder parameters on a method that is called in 1000 places. It’s potentially error prone in certain cases. Imagine the following:

class C {
     void Foo(int i, string s) { ... }
}

If we reorder the parameters of Foo, it’s simple to recompile and just fix up everything that breaks. However, what if you have:

class C {
    void Foo(int i, int j) { ... }
}

If you reorder that you better make sure that you find all the places that call that and fix them up, otherwise you’re going to have a bug. These types of reorderings are especially dangerous because after the reorderings your code continues to compile just fine.

A final example is the one I showed here at Tech-Ed. It’s dear to me because it’s something that bit me at one point. I started with the following code:

class BinaryNode {
     private BinaryNode left;
     private object value;
     private BinaryNode right;

     public BinaryNode(BinaryNode right, object value, BinaryNode left) {
           //initialize fields
     }
}

Now, at one point I realized that I wanted to reorder the left/right arguments to the constructor. I knew everything would still compile, so I did a find for all instances of “new.*BinaryNode” and fixed them up to call in the right order. However, i still ended up breaking something and it took quite a while to figure out what. Can you guess what it was?

The basic issue was that I had subclassed BinaryNode at one point and forgotten that I had a call to base(…) in the constructor. It was this sort of issue that using the re-order parameters helps you prevent. By using us we make sure that any code that calls the thing you’re reordering will be fixed up for you.

If you have more than 5 things calling your constructor/method, do you want to manually fix them all up, ptentially making mistakes along the way? Or would you prefer something that goes ahead and does it all automatically for you. If it’s the former, why? What do you prefer about manually modifying code. Are there issues with trust when it comes to modifying code? Do you want to actually see what’s getting changed? I know I sometimes do. By going and fixing up every location I get a good feel for the code base. For example, by routinely traversing the code (even though it’ painful), I can sometimes spot bugs, or opportunities for refactoring.


Comments (4)

  1. Paul Holden says:

    "By going and fixing up every location I get a good feel for the code base. For example, by routinely traversing the code (even though it’ painful), I can sometimes spot bugs, or opportunities for refactoring."

    This is very much how I feel. I see refactoring as an iterative process. Applying one set of refactorings often leads to the possibility of further refactorings, and you have the advantage of staying "fresh" with the rest of the codebase at the same time.

    Having said that many refactorings can be highly error prone, and it takes a lot of double-checking and testing to make sure you’ve not just introduced a bunch new bugs. The idea of automated refactoring is hugely appealing to me for this very reason, so I can’t wait to see how this develops.

    Is there any word yet on whether refactoring will be available for C++? Everything I’ve read to date suggests it’s a C# feature.

  2. Paul: One thing to note is that with every single refactoring we provide we give you the option to "Preview" the changes. This gives you a two paned view. The top pane shows you all the files that we’re going to modify. Clicking on any file then shows you all the changes we’re going to make in that file in a lower pane.

    That way you can use our refactoring to safely make all the changes, but you can also review the changes that are going to be made in order to get a feel for the code. Hopefully that will help.

    I don’t have any ideas about C++ and refactorings. I know there aren’t any in the community preview which we just released. Hopefully you can find a C++ blogger to ask.

  3. Bart Jacobs says:

    Something interesting when reordering parameters is that you should watch out with argument expressions that have side-effects.

    Consider the following original code:

    class C {

    void Foo(int x, int y) { … }

    int Bar1() { Console.WriteLine("Bar1!"); return 1; }

    int Bar2() { Console.WriteLine("Bar2!"); return 2; }

    void Test() {

    Foo(Bar1(), Bar2());

    }

    }

    Suppose I do a "Reorder parameters" refactoring on Foo. Of course, I want to preserve the original behavior. Therefore, the following is incorrect:

    class C {

    void Foo(int y, int x) { … }

    int Bar1() { Console.WriteLine("Bar1!"); return 1; }

    int Bar2() { Console.WriteLine("Bar2!"); return 2; }

    void Test() {

    Foo(Bar2(), Bar1());

    }

    }

    The following is correct:

    class C {

    void Foo(int y, int x) { … }

    int Bar1() { Console.WriteLine("Bar1!"); return 1; }

    int Bar2() { Console.WriteLine("Bar2!"); return 2; }

    void Test() {

    int x = Bar1();

    int y = Bar2();

    Foo(y, x);

    }

    }

    Is this how it’s done in Whidbey?

  4. Bart: Nope. But it’s an excellent point. We should probably warn that reordering parameters could cause the order of some operations to change. I don’t have the C# spec in ffront of me, but I do believe it specifies an left-to-right ordering in how arguments are evaluated. By reordering them you are necessarily changing the semantics in that regard.

    This is similar to rename and reflection. Say you’re using reflection to instantiate a type based on a string. If you rename that type then the instantiation will fail. This is because, like it or not, names of things have semantics that matter in the reflection domain.