One of my absolute favourite communicative refactoring is extracting a comparison to a method. Let me give you an example:
[csharp]if (activity.GetChanges() != null)[/csharp]
Instead, you could write:
[csharp]if (HasChanged(activity))[/csharp]
This gives me a couple of benefits. First, I don’t have to remember that activity has a GetChanges method that returns null when nothing has changed. This might not seem like much, but to me it’s important to make the code readable to humans, not compilers.
Secondly, I don’t have to think about the negation. The bang-equals sign is not natural for us. I have no scientific material to back this. Just trust me – negation is hard for us to understand. In human language, and very much so in code.
Third, I don’t have to use magic numbers. It might not be obvious, but here, null is a magic number. You have to remember that null has a special meaning.
Also, one of my favourite refactoring tool features is ‘reverse conditional’ that swaps the if and else blocks by iverting the conditional statement. As a side bonus, often when you see the inverted logic you get to go ‘wtf? that can’t be right? less than?’ and catch yourself a case-bug.
Left by Stefan Andersson on March 24th, 2006
Yepp. I agree. More readable, but I would prefer encapsulating the logic within activity:
if (activity.HasChanged())
Which to me looks even more readable in an OO manner. Can someone give me that refactoring tool?
Left by Tobias Fjälling on March 25th, 2006
Tobias:
In this example that I took from a real application, activity is a DataSet, which makes it kind of hard to change. But if activity was of a class that I controlled, I fully agree with you.
-andrés
Left by Andres on March 27th, 2006
I agree with comments 2 and 3 about putting HasChanged() on the class of the activity object (if possible). If you can’t modify that class (such as DataSet), and HasChanged() and its kin are showing up throughout your code, then consider the Decorator or Adapter pattern to put those methods on another class that wrap the DataSet.
In response to comment 2, Resharper will get you from the initial code in 2 refactorings.
Step 1: Highlight the original conditional and Extract method. This should create the following method in the same class as the containing method.
private static bool HasChanged(DataSet activity)
{
return activity.GetChanges() != null;
}
Step 2: (Let’s pretend we can modify the DataSet class.) Put the text cursor on HasChanged and choose “Make Method Non-Static” which will move the method onto DataSet as such…
public bool HasChanged()
{
return GetChanges() != null;
}
…plus all references to the original method will be updated!
Left by Jim Bolla on March 23rd, 2007