Archive for the 'Refactoring' Category
Best Links of the Week – Mar 27th 2010
More good links to share with others.
- Ouija Board Estimation\Seance Sizing – a new method for estimation that relies on team, not the undead.
- Is the Agile Community Being Unreasonable? – InfoQ takes a look at the friction between the PMBOK and Agile principles.
- Toyotas’ Journey from Waterfall to Lean Software Development – Henrick Kniberg takes a visit to Toyota and peaks under the hood to see how a Lean company develops software. What he finds will surprise you!
- Defining the Last Responsible Moment - Karl Scotland puts some meat on this fuzzy Lean concept by looking at the cost vs benefit of delay.
- Managing vs. Coaching vs. Mentoring - Jurgen Appelo makes the distinction between these three concepts.
- The Problems With Acceptance Testing – thoughtful entry by Jim Shore reevaluating the importance of automated acceptance testing on Agile projects.
- Alternatives to Acceptance Testing – more from Jim Shore on what can be used in place of automated acceptance testing.
- More on Automated Acceptance Testing - George Dinwiddie adds his perspective to the topic of automated acceptance testing.
- The Path to Frequent Deployments – a report from Kent Beck, author of Extreme Programming, on how to increase development speed by moving from annual to quarterly to monthly to weekly and finally to daily deployments.
- How to Be a Great Tech Leader – Richard Kasperowski talks about the technical elements needed to succeed when running an Agile project.
Best Links of the Week – Feb 22nd 2010
Links to share with your friends and co-workers
- Is an Agile PMO possible? – Curt Finch talks about the values of both Agile practices, PMI standards and how to marry the two.
- Self-organization: the secret sauce for improving your Scrum team – In this 90-miute video for Google, Jeff Sutherland talks about the role of self-organization and other advanced Scrum techniques.
- The Agile Flywheel – a short experience report describing how one company melded Scrum with their mature ITIL processes.
- Just do it: a quick intro to Agile’s technical practices – a summary of the technical core of Agile software development by Abby Fichtner.
- I love pair programming – Discussion on the effectiveness and the challenges of pair programming after a five month trial.
Is Code Foreclosure In Your Future?
I want to use this post to talk about the concept of design debt (or technical debt) to describe the variety of “shortcuts” software developers take to compromise design quality in order to gain a short term boost in productivity. While there may be many good reasons to take these “shortcuts” at the time, from that point forward any future design changes to that section of the code are more difficult, i.e. expensive to make. In almost all cases, most well-meaning software developers really do intend to return to the code and fix these shortcomings, but due to schedule pressures they rarely do. The cumulative effects of these “shortcuts” are called design debt and are very toxic to your software.
Much like credit card debt, a small amount of design debt is acceptable. Unfortunately, due to the nature of software, if real attention is not spent “paying back” the design debt, eventually the system in question collapses under the weight of its highly coupled design. I created the list below to help you spot the four stages of design debt in your system, so to avoid a “repo” of your software.
-
Impulse Buy - Not doing the right thing today or having the attitude of “I’ll fix that tomorrow”.
-
Late Payments - Adding new features to new code begins to take longer and longer.
-
Notice of Default - The project stops adding new features to enter a “refactoring phase” or “just a week for clean-up”.
-
Foreclosure - It simply becomes easier to re-write the system rather than modify it.
As you get into later half of Stage Three, more and more of your resources go into paying back debt and you cannot extend the functionality of your system to keep up with market demands. The competitive advantage your software provided the business falls to the wayside as all your time is spent in maintaining a brittle system your customers don’t even like anymore. Like most ailments in life, the longer you wait to apply a remediation, the more “painful” the cure. It is far more economical to apply an ounce of prevention, by taking your vitamins in the form of daily refactoring, than wait for the “foreclosure” process to overcome your business assests.
Extract Class & the Single Responsibility Principle
Last week we talked about Extract Method and why it is so important to know and love. This week, we are going to discuss another very important principle in object-oriented design and how Extract Class can help us build and maintain better code.
If you are familiar with Robert Martin’s book Agile Software Development: Principles, Patterns and Practices, you will recall he recast a very powerful concept (the Single-Responsiblity Principle or SRP) know from the “old days”, aka 1979,: a class should have one, and only one, reason to change. Or as I like to add my own spin to it: a class should only do one thing. The reasoning behind the SRP is if your class has two responsibilities, invariably they become coupled (coupled code == bad) and changes in one will impair or inhibit the ability of the class to meet the other responsibilities – and this is only in the case where you have just two responsibilities. Imagine the confusion when we have three or four responsibilities in a single class!!!
If SRP is so important, then why are talking about Extract Class refactoring? This is the refactoring you use when you want to apply the SRP and applying the SRP is key to writing good code, i.e. code that is testable, decoupled, maintainable, etc., etc.
How Do I Find It?: You have one class doing the work that should be done by two.
What Do I Do Once I Find It?: Create a new class and move the relevant fields and methods from the old class into the new class.
Any guidelines you care to share? I am glad that you asked because I do have a few tell tale signs that a new class is waiting to bust out of your old one:
- Find your big classes -> any class with more than a 1000 lines is probably doing more than one thing.
- Classes that have “too many” methods or fields\properties -> if class has more than 10 to 15 methods or fields\properties, you are probably missing an abstraction.
- Methods that don’t belong -> every class has a reason for existence, or a theme, that unifies the method and data. Methods or data that don’t “fit in” with the theme are likely candidates for a new class.
In an application I am working on, we have a real simple Bus class that shepherds messages between simulated hardware and simulated devices (in our case we are talking about drawers, buttons, LED, etc.). When you examine the class you see it is about 250 lines, has three fields, a number of events around connecting and disconnecting devices and some private methods. Looking further, you can find two methods which stick out and don’t match the theme of the class:
45 private IBusMessage BytesToBusMessage(byte[] bytes)
46 {
47 // convert to string
48 string msg = Encoding.ASCII.GetString(bytes, 0, bytes.Length);
49 // split the msg into address and actual command
50 string[] splits = msg.Split(new char[] { '|' });
51 BusMessage message = new BusMessage();
52 message.Address = long.Parse(splits[0]);
53 message.Message = splits[1];
54 if (splits.Length > 2)
55 {
56 message.SequenceNumber = int.Parse(splits[2]);
57 }
58 return message;
59 }
60
61 public static byte[] BusMessageToBytes(IBusMessage message)
62 {
63 // form the message
64 string msg = message.Address + "|" + message.Message + "|" + message.SequenceNumber;
65 return Encoding.ASCII.GetBytes(msg);
66 }
67
So we have a class which is responsible for event registration AND parsing messages. My question is this: why in the world does a class which mostly deals with event registration have anything to do with parsing? Why would this class even care how we parse messages back-and-forth? It sounds like a class doing more than one thing – that is a violation of the SRP – and we will use Extract Class to apply the SRP to Bus.
Next week, we will talk about the Move Method refactoring, which is how you extract your class from the old one.
Extract Method – The Most Important Refactoring EVER
Coming off my last rant, I thought it might be valuable to the community if I were to blog on the various refactorings I felt software developers should be using if they consider themselves professionals. To that end, I am going to borrow the refactoring patterns from Martin Fowler’s book, but provide up-to-date examples in C# code. To start the series off, I will start with the one refactoring ever developer should know and love – Extract Method.
How Do I Find It?: You have a code fragment that can be grouped together.
What Do I Do Once I Found It?: Turn the fragment into a method whose name explains the purpose of the method.
Fear not if you have never heard of this refactoring. It is so fundamental that many developers execute this pattern as a matter-of-course, but did not know the name for it; well, now you do – it is called Extract Method. IMO, mastering this refactoring is the key to understanding the later refactorings, like Extract Class, which we will discuss next week.
So, how do you located code that is a candidate for this refactoring? Here are some guidelines I use:
- Long methods -> methods that exceed 20 lines are a good candidate for this refactoring since they are normally doing more than one thing.
- Code comments -> comments are normally a visual marker that the next lines are conceptual similar and this refactoring is about grouping conceptual similar things in methods.
I randomly searched through our source code repository at work and in a mere 24.4 seconds I found this sample code from a utility class (Util.cs). It is a nice method that does a useful thing (removes extra decimal points from a string – who knew you needed to do that, but why not?), and it needs some help based on my guidelines above:
44 static public string RemoveExtraDecimalPoint(string text)
45 {
46 string returnString = "";
47 int countDecimal = 0;
48
49 foreach (char character in text)
50 {
51 if (character == '.')
52 {
53 countDecimal++;
54 }
55 }
56
57 if (countDecimal > 1)
58 {
59 bool first = true;
60 foreach (char character in text)
61 {
62 returnString += character.ToString();
63 int length = returnString.Length;
64 if (character == '.')
65 {
66 if (first == true)
67 {
68 first = false;
69 }
70 else
71 {
72 returnString = returnString.Remove(returnString.Length - 1, 1);
73 }
74 }
75 }
76 }
77 else
78 {
79 returnString = text;
80 }
81
82 return returnString;
83 }
What makes this a good candidate for Extract Method is this method us 39 lines long – WAY too long for a method if our target is to create method from 5 to 10 lines long (which is the target of this refactoring). Remember, one of the main goals of Extract Method is to create small methods with intention revealing method names that read like comments. Our main focus will be to turn this long method in to a series of method calls which tell the reader what happens inside RemoveExtraDecimalPoint.
The first step after identifying a candidate method for Extract Method is to check the unit tests for this method and make sure they run. Surprise, surprise – this method has no tests!! Just a polite reminder – never, ever refactor code without having either an automated unit test or integration test confirming you did not break anything. That is called hacking. Yes, there will be times you have to “hack” something in, but while learning how to refactor is not it. Writing a unit test after-the-fact is called a characterization test. Here is one of my unit tests (I actually wrote five tests) that will serve as my safety net during the refactoring:
22 [TestMethod]
23 public void RemoveExtraDecimalPointTest()
24 {
25 // setup
26
27 // execute
28 string test = Utils.RemoveExtraDecimalPoint("23..45.0.");
29
30 // verify
31 Assert.AreEqual("23.450", test, "Extra decimal points not removed.");
32
33 // teardown
34 }
My very first Extract Method refactoring will focus on line 47 in the original sample code. The code below shows the new method call I made for this refactoring:
44 static public string RemoveExtraDecimalPoint(string text)
45 {
46 string returnString = "";
47 int countDecimal = Utils.CountDecimalsInString(text);
48
49 if (countDecimal > 1)
50 {
The Extract Method refactoring product a method 10 lines long and has an intention revealing name.
77 private static int CountDecimalsInString(string text)
78 {
79 int countDecimal = 0;
80 foreach (char character in text)
81 {
82 if (character == '.')
83 {
84 countDecimal++;
85 }
86 }
86 return countDecimal;
87 }
We run all our tests, everything passes, we are fine! Time to move on to find another chunk of code to extract. Next we take a look at line 59 from the original code and use Extract Method to make the following new method.
89 private static string ParseExtraDecimalPoints(string text)
90 {
91 string returnString = "";
92 bool first = true;
93
94 foreach (char character in text)
95 {
96 returnString += character.ToString();
97 if (character == '.')
98 {
99 if (first == true)
100 {
101 first = false;
101 }
102 else
103 {
104 returnString = returnString.Remove(returnString.Length - 1, 1);
105 }
106 }
107 }
108 return returnString;
109 }
Again: run all the test, everything passes, we are fine! Notice that our extracted method is a little long (20 lines), but conceptually all the code in ParseExtraDecimalPoints is related, so we are OK. Remember, we are more interested in finding related code than following a rule of “10 lines or less”. Even if all you can pull out is a 200 line method, then that is cool – it is a first start once you pull one chunk out, you can begin to see the smaller chunks embedded in there.
But what does the refactored method look like? I highlighted the addition of the two static methods – CountDecimalsInString and ParseExtraDecimalPoints – to make it easier to see our changes.
44 static public string RemoveExtraDecimalPoint(string text)
45 {
46 string returnString = "";
47 int countDecimal = Utils.CountDecimalsInString(text);
48
49 if (countDecimal > 1)
50 {
51 returnString = Utils.ParseExtraDecimalPoints(text);
52 }
53 else
54 {
55 returnString = text;
56 }
57
58 return returnString;
59 }
Upon examination, we see the refactored version of RemoveExtraDecimalPoint is only 15 lines long and does just two things: counts decimal points and parses code to remove extra decimal points. Oh, that is real easy to understand as opposed to trying to read the original 39 lines of code (take a look if you don’t believe me) with all the loops, conditionals and white space and trying to figure things out.
And that is the point of the Extract Method refactoring.
A Rewrite is NOT Refactoring!
Just the other day I was listening to, i.e. eavesdropping, a conversation between peers where they were trying to decide on what version of Visual Studio.NET their team should migrate to. This team was still using Visual Studio 6 (!!) and wanted to know if they should go to 2005 or 2008. During the conversation the motivation for the upgrade was discussed:
“We want to refactor some of the old [C++] code in C#.”
[OK, now imagine the sound of metal scraping on your fillings. It is that bad.]
Refactoring has been in print since 1999 and has a really clear definition. I also tend to think it is one of the fundamental skills every software developer should be proficient in. Frankly, I feel if you are not refactoring your code, you are not doing your job. So, why do people in this day and age of our profession still consider a rewrite of old code in a new language refactoring? Just another example of sloppy language and, unfortunately, sloppy thinking. So, to be precise, here are the definitions of refactoring.
refactoring (n.): a change to made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior.
refactoring (v.): to restructure software by applying a series of refactorings without changing its observable behavior.
For those of you who have stumbled upon my rant, here is a short list of things I do NOT consider refactoring:
- adding new functionality to the code.
- changing the design in the absence of automated tests – either unit, functional or acceptance.
- a separate “stage” in the project you do either after, or before, the “real” work.
- a replacement, or substitution, for a genuine software design.
- “fixing things up while you are in there.”

