Xp Refactoring Pitfalls

XpPitfalls

Problem: Refactoring is hard. Really hard.

• Solutions
  • Practice, practice, practice.
  • Pair promiscuously.
  • Read.
  • Pair with people outside your team, either on a side project or by doing an "exchange student" deal with another company.
  • Take a [hands-on] class.
  • Get an automated refactoring tool. We use IntellijIdea, for example. It won't do all the work, but it will do most of the boring parts.

Problem: Refactoring depends on high-level OOAD / Design Patterns skill and experience.

• Solutions
  • Focus on duplication.
  • See solutions to above ("Refactoring is hard.")
  • Blindly remove duplication from everywhere. The resulting code is usually better. You will learn over time when it doesn't work out. Remember that the goal in the beginning is to learn and that better code is a side-effect. Pay special attention to names: if you see common prefixes, extract a class or an interface. -- JbRainsberger

Problem: Large refactorings. Refactorings that feel like they should be easy yet take a long time; you could end up getting only halfway done, which means your code is worse that it was at the start (since now it expresses the same thing in two different ways).

• Solutions
  • Be creative about finding smaller, yet consistent refactorings.
  • Set a timer for 20 minutes; if you're still getting started or still trying to figure out what to do, give up and try again with something smaller.
  • Break the refactoring into small pieces, and then after each piece is done, re-evaluate whether you need to continue.
  • RefactorLowHangingFruit.

Problem: Customers hate refactoring, since it doesn't add value. They will press you to add more features. (Note that this pressure can actually be good! It can help keep you from thrashing (see below).)

• Solutions
  • Refactor a little at a time, all the time.
  • Make sure big refactorings are bounded, and have a specific performance goal.
• Anti-solutions
  • Don't ask the customer for permission to refactor.
  • Don't tell the customer that you need some time away from the stories to do some refactoring.
  • Don't write "refactoring stories".

WillemBogaerts's response:

• "Don't ask the customer for permission to refactor" I see refactoring as part of my job. If a bicycle maker needs to drill out a hole in the frame because you requested him to add another part to your bicycle, he will do so. If I need to add a piece of code and need to adapt the existing code to this extension, I will do so as well.

Problem: Thrashing: endlessly refactoring the same code to be marginally better (or just different) without contributing to velocity or to business value.

• Solutions
  • Try to make every refactoring with a specific story or task in mind (maybe only in the back of your mind); hoping that as soon as you're done with this refactoring (and maybe just one or two more) you can finally implement the task.
  • Discuss the refactoring with the team so see if other people think that you're not going to get much out of the refactoring.
  • Work with a partner. Your partner should be able to tell you when you're gold-plating.

Problem: Existing large unclean (legacy) code bases are really hard to refactor.

• Solutions
  • Consider ditching RF, and instead relying on building unit tests and/or starting from scratch? (AlexChaffee: On a recent project with a large legacy base, I feel we would have made better progress starting over. This is very hard to measure so it's difficult to say which would be better. It's also difficult to know whether you captured all the business logic that was nested inside the tangled legacy code.) (ErikHanson: Starting over without a really good customer test suite sounds scary. I fear that's about to happen on my project.)
  • Lay down a set of functional tests first. Functional tests, otherwise known as "black box" or "input-output" tests, are at a higher level than unit tests. They test an entire component by providing a set of sample inputs and validating the expected output. Functional tests are not a replacement for unit tests -- they are coarser grained, usually take longer to run, are more difficult to read and understand, and tend to miss edge cases. However, they are a vast improvement over having no tests at all, and can help jumpstart the refactoring/unit testing process.
  • Small legacy code can be refactored by adding unit tests. Automated tools also help a great deal here. Large modules of code usually take more effort than starting over. Either use them as is, wrapping them if necessary to modify behaviour, or if this is not an option, start over. Nobody has a good solution for this problem, so the fact that XP doesn't either isn't really a pitfall of XP, it's a pitfall of working with unclean legacy code.

Problem: Refactoring and Collective Code Ownership can cause disorientation. People may sit down and see code that looks completely foreign. This may require people to re-learn the whole system, or (worse) to make them feel like they have to relearn it when all they want to do is write some code. They get a sense of nausea and vertigo when sitting down to look at code, and there's no high-level view, and this hurts morale and productivity.

• Solutions
  • automatically generated object/architecture diagrams
  • Sketch a rough object model diagram as soon as you sit down with unfamiliar code.
  • Ask for help from someone else who understands the component.
  • Refactor to understand.
  • Do far-reaching refactorings with a projector and the whole team.[Has anyone tried this?]
  • When you're done, take a few minutes for a show-and-tell.
  • Browse the unit tests to understand the code. Refactor the unit tests to improve their clarity. -JimKingdon?
  • Work with a partner.

Problem: OAOO can lead to Demeter gone mad. You could end up bloating your interfaces.

• Solutions
  • Be aware of the possibility and try to avoid it.
  • DemeterGoneMad? is a sign that you need to reorganize your objects' responsibilities: the solution to foo.getBar().getBaz().bing() isn't simply wrapping that call in 'foo.bingTheBazFromBar()', or 'foo.bing()' for that matter, but moving the functionality from whatever 'getBaz()' happens to return to 'foo'.

Problem: OAOO can lead to a nest of small objects. Writing new code or debugging you end up getting lost in a maze of pointers.

• Solutions
  • Stop the madness. Trust your design instincts.
  • Solution: All of the XP mottos -- OAOO, YAGNI, etc. -- must not prevent you from following the motto "Don't Be Stupid."
  • Don't hire former Smalltalk programmers :-) (Especially since if you do, you risk seeing how smart they are and turning into one.)

---

CategoryRefactoring