Extract method to class (part 2)
Check out the previous post for some context on this.
Let’s start with a simple example and walk step by step through the refactorings we’d have to provide to make this easy. Rather than one uber step it would be nice to have a lot of little baby steps that you could perform easily to get to your final goal.
My starting code looks like this:
public TextSpan ValidateBreakpointLocation(long line, long column)
{
//Get all the state ready
IParseTree parseTree = this.ParseTree;
IMemberParseTree memberParseTree = this.MemberParseTree;
ITokenStream tokenStream = this.TokenStream;
// lots and lots stuff...
}
1) Extract Method into class.
a. Choose new class name: i.e. BreakpointValidator, or have us automatically choose something like ValidateBreakpointLocationImpl
b. Choose new method name: i.e. Validate, or have us chose something like “Work”, “Run” or “Execute”.
c. If the method body accesses anything private to the current type then the new class needs to be nested inside the current type. Otherwise it can a sibling type
d. If we picked the names automatically and you didn’t like them, then rename at will
e. The new class will look like this:
class Source : IVsLanguageDebugInfo
{
class BreakpointValidator
{
Source source;
long line;
long column;
internal BreakpointValidator(Source source, long line, long column) {
this.source = source;
this.line = line;
this.column = column;
}
internal TextSpan Validate() {
//lots and lots of stuff
}
}
public TextSpan ValidateBreakpointLocation(long line, long column)
{
return new BreakpointValidator(this, line, column).Validate();
}
}
f. Things to note: if there are out or ref parameters to the original method they get passed to the worker method.
g. The worker method is patched up to contain the old code of the original method. However, references to “this” are now made into references to “source”
2) ‘Encapsulate local into field’ inside “Validate”
a. Choose any local inside the method and ask to promote it to a field. You can pick a new name or default to the current name in the code (as long as it doesn’t conflict with an existing field name). So you transform the following like so:
internal TextSpan Validate()
{
IParseTree parseTree = source.ParseTree;
IMemberParseTree memberParseTree = source.MemberParseTree;
ITokenStream tokenStream = source.TokenStream;
//lots and lots of stuff
}
class BreakpointValidator
{
Source source;
long line;
long column;
IParseTree parseTree;
IMemberParseTree memberParseTree;
ITokenStream tokenStream;
internal BreakpointValidator(Source source, long line, long column)
{
this.source = source;
this.line = line;
this.column = column;
}
internal TextSpan Validate()
{
parseTree = source.ParseTree;
memberParseTree = source.MemberParseTree;
tokenStream = source.TokenStream;
//lots and lots of stuff
}
}
3) ‘Extract method’ inside “Validate”
a. Perform the current extract method refactoring we provide (as well as maybe some other refactorings) until you get the code in the way you want it. Your methods are no longer hideously ugly because the state they need is now made available to them without cluttering up either the method signatures or the state of your original class.
We already provider the third refactoring, so in order to enable this kind of work we’d need to do the first two. The first seems pretty simple (although I’m sure I’ve probably forgotten some case that makes it hard). It’s just a “find references” on the “this” object to see how it’s used and some additional code gen. The second refactoring also doesn’t seem so bad and I think could be done rather easily.
In the end you’ve taken a complicated method (or set of methods) which have to store and pass around a whole lot of state and you’ve teased it into a specialized class that is hopefully a heck of a lot simpler. I'm probably missing a lot of issues that would probably arise if we tried to implement this, but i do think it's well worth it. This is a pretty common activity that I do (especially when dealing with a lot of our old legacy code) and it would save me a lot of time to have these little steps done automagically.