Whidbey Refactorings: Signature Change

Almost 4 months after the first post in the series… not bad!

There are 3 Signature Change Refactorings:

• Remove Parameter
• Reorder Parameter
• Promote Local

I rank these as “Tier 2” – they’re not as important as Extract Method and Rename, but they’re still pretty hard to do by hand. They also have some complexity that can’t be resolved easily, making them slightly less reliable than the Tier 1 Refactorings.

The first attempt

Our first attempt created one feature called Signature Change. There was a single menu item & a single piece of UI. You were to invoke them on a method in your code, showing a form you could interact with. There you would party on the signature for a while, then commit all your changes.

This approach is very Microsoft – build a full-featured tool that doesn’t just enable a scenario, but covers every contingency we can anticipate. Think “one stop shopping”. The form had a lot of buttons & stuff. It was huge.

It was also wrong.

It didn’t match the Elvis attribute of being “code focused”. That is, we think that the C# customer is more comfortable manipulating code in an editor than in a form. (Contrast with Mort, who prefers forms to code.) This is the same reason we removed things like “Add Method” in the class view.

Once you were in this UI, it was only natural to expect it do even more, like rename the method or a parameter. This continued to add to the complexity of the UI & hurt usability.

It also didn’t match the way people really manipulate method signatures. You don’t look at a method and say “this method has 5 things wrong with its signature, and now I’m going to fix them”. You’re more likely to say “this method needs a new parameter for the work I plan to do” or “this method works correctly, but doesn’t match my coding convention of having all ‘out’ params at the end of the argument list”. That is, sig. change Refactorings are always small single steps and the UI we built was optimized for big steps (really multiple small steps).

As you can see, all this added up to it being the wrong UI, at least for Elvis (the typical C# customer). It was a very typical Microsoft way of building a feature, so it took us a little time to get on the right track.

As a special case of the “code focused” bit, it really worked poorly for “Add Parameter”. You click on the button and get a small dialog with 3 fields (name, type, default value). You then get to a fully qualified type name into an edit field. That’s no fun, especially since typing in the editor gets you a familiar, well-tuned IntelliSense experience. Finally, the name “default value” was confusing, because it seemed to suggest C# language support for default parameter values like those that exist in C++ and VB.

The second try

So, we took the UI and tore it apart. In the editor you can now click on a method (at the call site or definition) and request a Remove Parameters or Reorder Parameters.

Reorder Parameters

A form appears allowing you to move parameters up & down on a list. Once they’re to your liking, you can apply the change.

This isn’t quite the ideal, code-focused model I would have liked. You’re acting on the whole method’s parameter list, but I think that acting on single parameters might have been better. Instead of being able to arbitrarily reorder all the parameters on a method at once, suppose we just supplied “move right” and “move left” operations on each parameter individually? Suppose they were extremely fast. That would probably be a simpler, more streamlined experience, and be more Code Focused.

Remove Parameters

Similar UI to Reorder, you can “delete” one or more parameters from the list.

Similar to my concerns about Reorder, I think that a model focused on removing a single parameter would be a better experience than this model, which allows you to remove multiple parameters at once.

Remove carries a small risk of changing the behavior of your code. Consider:

     F(i++);

Removing the parameter will change the behavior of your code. We considered building more smarts protect against this, and then decided not to solve the halting problem for Whidbey. Instead, we put a warning on the UI that this Refactoring could change the behavior of your code. You can use the “Preview Changes” option if you’re worried. (Personally I avoid coding styles that allow this problem, by differentiating between “query” and “action” methods, but we still need to make sure that we’re building tools for our customers, not for ourselves.)

Similar issues are possible in the Reorder case, but even less common, so we decided not to clutter the UI with a warning in that case.

Invocation Model

For both Reorder and Remove, you can invoke them via the context menu or the top-level Refactor menu, at either a call site or the definition. This brings up a modal form to manipulate the selected method.

You can also invoke these 2 Refactorings using “smart tags”. We put one on the definition of each method in your code, as a way of indicating “there are special things you can do here”. For both Refactorings, the smart tag offers Reorder and Remove actions.

Aside: Users seem mildly annoyed by smart tags in Office usability studies. If they had a more positive reaction there, you would have seem more aggressive use of smart tags in VS. For example, we could place a smart tag on every definition, of fields, locals, parameters, types, methods, etc. Most would offer at least rename, and each might offer other cool stuff. For example, the smart tag on a parameter might also offer the “Move Left” and “Move Right” actions I mentioned previously.

Smart tags are a lot like context menus, but even more focused on the location & should provide greatly added value, compared to the context menu & other sources of actions.

They seem to solve a certain class of discoverability & clutter issues. For example, suppose we decided to put “Move Left” & “Move Right” on the context menu. If we only show them in the menu when you click on a parameter, then users will see it once without realizing what the context was, and then get frustrated trying to find it again. If we always show it on the context menu, no matter where you click, then the context menu quickly becomes cluttered.

These are the things we spend our days fretting over!

Add Paramters

Where’s “Add Parameter” in all this? Well, it’s not in Whidbey. I was convinced that there wasn’t a good user model for providing this Refactoring, at least with the facilities we had available at the time.

The problem is that no one wants to type type names into dialogs. It’s just no fun. Completion lists are where it’s at. (We talked about showing a completion list on the dialog, but morphing the code to support that seemed pretty expensive. I’m also not convinced that it would be an ideal experience.)

There was also this really odd bit where you had to supply a default value to pass in at all existing call sites. I already mentioned that the name “default value” was confusing because that name already refers to something in C++ and VB. It also caused problems because typing in the default value in an edit box isn’t ideal, either, like the type name. If you’re going to pass anything other than a simple constant, you really want to have IntelliSense helping you out. (Imagine passing “new SomeType(param1, param2)” – you’d want parameter help on that constructor, right?)

Really, the notion of typing this stuff in to a form just doesn’t fit with the code-focused nature of the typical C# customer.

We decided to just cut the feature for Whidbey, and come back to it later.

Promote Local

Then we came up with a cheap alternative that seemed to add a lot of value. Suppose you have a method with this line of code in it:

        bool b = true;

Notice that it has exactly the right data for “Add Parameters” – name, type, value. We could build a Refactoring that would take an initialized local variable & “promote” it to a parameter. The type & name of the new parameter are exactly that of the local variable. The value to pass in at all the call sites is the value on the left of the equals. You got to write the type & the value using your favorite tool for the job (the C# editor, of course!).

What I’m talking about doesn’t solve all “Add Parameter” needs in the way we would have liked. It wasn’t clear that users who were looking for a way to add a parameter would think of trying this, and we wondered if users would just be plain confused by the whole idea. We also looked at the schedule and saw that there wasn’t much time to fit something like this in, especially if it wasn’t going to be the “ideal” solution.

We decided to take the cheapest approach possible. If people don’t like the feature, it wouldn’t be a big loss. It would fit in without causing the schedule to slip. So we first applied a set of constraints:

• The value on the right must be a constant. This way we know it can be passed in legally at all call sites.
• The value on the left can’t even be a constant expression (“1 + 1”), because analyzing that sort of thing could be hard.
• The variable must be initialized at creation time. So reject “int i; i = 1;”.
• The new parameter goes at the front of the parameter list.
• No options. There’s just one way to do it, and that’s it.

Looking back, I’m really glad things worked out this way. It’s very XP-like. We took the smallest step that delivered the greatest value to our customer. If we did this more of the time, we would ship a lot more often!

As XP-ers know, building working software generates a lot of learning. As soon as we started exploring Promote Local, we saw that it needed a bit more flexibility to really be useful, so we changed the constraints somewhat:

• If the value on the right isn’t obviously a constant, we’ll warn that it may not be the right thing to pass in at all call sites, but still allow it. For example, if the value includes a local variable, things aren’t likely to turn out well.

This allows some really important cases, like “Foo f = new Foo(“hi”);”. Note that we don’t fix up any type names on the right side. We just take the text in your code, as typed, and copy it to all call sites. It’s the simplest approach I could think of!

In the normal Microsoft way of thinking, we might have decided to build up a lot of logic to decide if the thing on the right was acceptable for the call sites. We could analyze visibility of any methods called, and offer to change their accessibility. We could offer to Encapsulate Field any fields that might be needed at the call site. We could fully qualify type names on the right, or offer to add ‘using’ directives as needed. But that would have been too expensive to deliver in Whidbey (we would have had to cut something clearly more important), so instead we would have cut Promote Local. But, applying more of the thinking we’ve learned from XP, we decided to continue to deliver this simple, cheap Promote Local.

I’ve heard people ask that we be smart about placing the parameter in the correct location (before all ‘out’, ‘ref’, and ‘params’ parameters), or offer UI that lets the user select the position of the new parameter. I argue that you’d rather accept the default of first position, and then later apply Reorder Parameters if you want to change things. This lets you take a smaller step & get back to delivering customer value sooner. (To be fair, there are many who disagree with me on this point.)

Promote Local is the only Refactoring we built that doesn’t have a form to take input. That means there isn’t a good place to put an option to Preview Changes. I get requests that we do something about this, but I really like the streamlined behavior we have today. Click on a local variable, invoke the command, and you’re done. That’s it. (Obviously, there are many who disagree with me on this point, too.)

Aside: Earlier I mentioned what we might have done with more smart tags. If we put a smart tag on all local variable definitions, one of the items on it would be “Promote to Parameter”.

Two other things about Promote Local are important to me:

• It’s written in C#. Most of the Whidbey C# IDE is written in C++. (Extract Interface and the various forms for other Refactorings are the other exceptions.)
• I wrote it. I spend a lot of my time managing people (oh, and blogging), so any code I get to ship is pretty special to me.

What else?

Missing from the picture are the “ret <-> out” Refactorings.

The first takes a method with no return type (“void”) and changes one of its ‘out’ parameters to be the return type.

     void F(out int x) //...

     F(out x);

Becomes

     int F() //...

     x = F();

Seems pretty cool, but there was a lot of complexity to overcome.

The second is the inverse (takes a return value and makes it an “out” parameter). This one is even harder to do, because sometimes the return value is the input to another method:

     G(F()); // how do we do ret->out on F?

These would have been nice to have when you use Extract Method; it removes the question of how to decide what to do with outputs from the new method (“all ‘out’ params? Select one to be return and others be ‘out’?”).

However, the value of these Refactorings is lower than a lot of other very cool stuff, and the costs were pretty high due to the complexity involved. So they didn’t make it in Whidbey.

In the case of all of these Signature Change Refactorings, it’s clear we could have done a bit more work to make them even cooler. But as I mentioned in the beginning, these are “Tier 2”, so we would rather spend less time on them in favor of our Tier 1 Refactorings.

Thoughts?

So, what do you think? Did we get it right? Will these Refactorings, as written, suit your needs?

Should we have stuck with the single form for all? Or continued pushing in the small steps direction? If we provided both models, which would you use?

Is Promote Local interesting and valuable as-is, or just a waste of time & UI space?

Do you need the ret<->out params more than we realized? Would you rather we cut something else in their favor?

For Beta 2, should we punt a set of bugs to make time to enhance these Refactorings?

If you want to discuss, comment here. If you have stronger feedback, then send it via Ladybug.

Comments

• Anonymous
  July 25, 2004
  Jay,
  I just had one small comment to make regarding your side note "Users seem mildly annoyed by smart tags in Office usability studies."
  
  I, too, have occasionally found myself annoyed at the smart tag usability in Office. On the other hand, JetBrain's IntelliJ IDEA (http://www.jetbrains.com/idea/index.html) uses smart tag-like functionality (they call them "intention actions"), and I find them incredibly useful, intuitive, and relevant. I've never had more pleasure using an IDE than I did with IDEA where it seems like every day I would discover something new with it, and it would perform the functionality exactly how I wanted it to.
  
  Code editing is entirely different than general office document work, and I find smart tags far more useful in the former. I was honestly surprised to hear that Office usability studies would impact the design of a code editor in this regard. I sincerely hope this decision is revisited...

• Anonymous
  July 25, 2004
  Very interesting, Sean. I'll bring it up with the team.

• Anonymous
  July 25, 2004
  small indvidual refactorings is the way to go...the less UI you need the better. The ideal is no UI at all (not that its achievable). This way you can refactor with a keystroke and not get held up by selecting many options for the refactoring. This is also a lot more decoupled!
  
  Keith

• Anonymous
  July 25, 2004
  The comment has been removed

• Anonymous
  July 25, 2004
  The comment has been removed

• Anonymous
  July 25, 2004
  Oops, I should pay more attention to what I'm reading... Disregard my previous post :)

• Anonymous
  July 25, 2004
  The comment has been removed

• Anonymous
  July 25, 2004
  Another great blog entry. I'm with Nicholas on the "how" not being clear until now - and it is always interesting to hear the "why" as well.
  
  [Will these Refactorings, as written, suit your needs?]
  
  Yes, I am loving refactoring in Beta 1

• Anonymous
  July 25, 2004
  The comment has been removed

• Anonymous
  July 26, 2004
  The comment has been removed

• Anonymous
  July 26, 2004
  I agree with Sean Timm: smart tags (ST) are the IntelliSense of context menus (or at least a thing between) so it does make sense to provide ST in a code editor.
  
  On the topic of this entry: now that I know about all this build-in refactoring fun I want it for C++ too :)

• Anonymous
  July 26, 2004
  I don't think the smart tags would be as bad if they didn't require so many actions to activate. To use them, I have to click on a function name (say), move the mouse, hover over the function name, hover over the icon, click on the arrow, and then click on the menu item. Seems a bit much.

• Anonymous
  July 26, 2004
  What David said - it seems like smart-tags could be combined in interesting ways with mouse-gestures. Grab the smart tag and move the parameter around. Grab the smart tag and move the local variable to a parameter. Grab the smart tag and throw the parameter away. Grab the smart tag...

• Anonymous
  July 26, 2004
  Larkware

• Anonymous
  July 26, 2004
  avoid dialog boxes while refactoring

• Anonymous
  July 26, 2004
  David,
  
  You can use Ctrl+. (or shift+alt+F10 which also works in Office) to bring up the menu, any time you are on an identifier with a smart tag.
  
  Saves a lot of using the mouse.

• Anonymous
  July 26, 2004
  The comment has been removed

• Anonymous
  July 26, 2004
  Kevin and Cyrus: thanks, that does help. But it's no excuse for requiring such a convoluted sequence to invoke the smart tag with the mouse. Especially because discoverability is central to smart tags, and that favors the more intuitive mouse interface.
  
  Also, why doesn't ctrl+. highlight the first item in the menu?

• Anonymous
  July 28, 2004
  Exactly what David says. Everywhere in windows, right-clicking the mouse on something brings up a "context menu".... Context being the key word -- the menu depends on what you right-clicked on.
  
  So why not put the "smart tags" in the context menu if you click on the item? For example, in whidbey if you declare a class as implementing an interface, the interface name has a smart tag. To do the useful "implement implicitly" action after I've stubbed out my class I have to click on the name, hover over the tag, click on the icon, choose a menu item. ack. can't I use the context menu? as far as always adding the items, why not varying the items in the context menu based on the (gasp) context the menu was brought up in?
  
  Here's a compromise: in the context menu of the code editor, add a "smart tag" sub menu, normally disabled. For items that have smarts on them, the smart tag provides the items under that submenu. Heck, just ask the smart tag to draw itself as if it were a submenu. Anything.
  
  Then I can just right-click, and choose "smart tag | implement implicitly" from the context menu. Lots of annoying steps removed.
  
  It would allow me to use the smart tag either as described above, through the keyboard (ctrl+.), or through just two clicks and a hover. This would make the code editor smart tags much, MUCH more usable (to me and the few people over here I "tested" the idea on). Any chances on that?

• Anonymous
  July 30, 2004
  As Jay suggested, I opened a Ladybug entry for this:
  
  http://lab.msdn.microsoft.com/productfeedback/viewfeedback.aspx?feedbackid=53ae5577-02c2-40e7-a186-c4aa89f537bd
  
  If you like the idea of usability enhancements for refactoring, go vote for the entry!

• Anonymous
  July 30, 2004
  That's a great way to go, Bruce!
  
  XP teaches the value of putting the decision making around feature set into the hands of customers.
  
  Customer asks for features. Developer does rough costing. Customer selects which features they want to pay for next.
  
  So, go to lady bug & vote!

• Anonymous
  August 03, 2004
  And three days later, the developer has closed the bug. If we're lucky, we'll only have to wait three years for improvements.

• Anonymous
  August 03, 2004
  David,
  
  You sound disappointed. As Anson notes, some of the feedback from this blog has already been incorportated into the VS2005 product. So you have had a real effect.
  
  Some of it has been postponed, primarily because it's too large a change to fit into VS2005.
  
  Customers have repeatedly told us that they like Whidbey the way it is, and they want it now! Any decisions that expand the scope of Whidbey just mean that it's even longer until you get the final product.
  
  I firmly believe that the best thing we can do is ship, and then ship again. With all the feedback and learning we get from VS2005, the next version is sure to be a much better product.
  
  That said, we're still plan to modify VS2005 features based on customer feedback. Some items have already been fixed, and some are still in the works. We look at every bug fix & suggestion. We look extra carefully at the highest-ranked suggestions, both by "rank" and count of votes.
  
  So keep the feedback coming, and keep voting.

• Anonymous
  August 04, 2004
  The comment has been removed

• Anonymous
  October 26, 2008
  Work from home moms. Moms work from home. Voyforums work at home moms. Work for stay at home moms. Moms work at home. Work at home moms message boards.

• Anonymous
  June 07, 2009
  PingBack from http://greenteafatburner.info/story.php?id=4868

• Anonymous
  June 16, 2009
  PingBack from http://workfromhomecareer.info/story.php?id=24816