Sunday, 16 March 2014

Code Quality - Refactoring


My project team has been asked to improve code quality by performing refactoring. I know that code refactoring is all about reviewing the code and ensuring that it follows the best coding practices. I however seek your guidance as to what are the aspects one should look into while performing code refactoring and also point me to some references, where I can learn more about the same.

By: Rajesh Dhawan


Your question has the answer to some extent. Martin Fowler and Kent Beck originally defined refactoring as "a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior… It is a disciplined way to clean up code that minimizes the chances of introducing bugs". Refactoring can be as minor as an inline change or it could be a massive code rewrite. Though large scale code refactoring can be an architectural redesign, as long as it does not change the business logic and when it is done in steps, it can be called as refactoring as well.

This is mostly done to improve the non-functional attributes of the program thus leading to improved code readability and maintainability. The design priorities or constraints for the enterprise applications are usually defined by the Enterprise Architecture teams, considering the vision, mission and goals of the enterprise and in turn the IT organization. Having said that while refactoring is all about making code changes to make it conform to the non functional attributes like maintainability, scalability, performance, etc, the specific design best practice or design pattern to consider would differ case to case.

Ideally, refactoring is done in iterations and technically there is no end to refactoring. However, one can say that refactoring ends when the code becomes 'simple', in which case, the definition of simple is subjective. As a programmer, you should be able to smell your code to figure out whether it needs refactoring. Here are some situations when your code smells, inviting you to refactor:

  • Duplicated Code - extract out the common bits into theirown method (extract method) if code is in same class if two classes duplicate code, consider extract class to create a new class to hold the shared functionality.
  • Long Methods - extract method!
  • Large Class - Class trying to do too much often shows up as too many instance variables.
  • Long Parameter List - replace parameter with method (receiver explicitly asks sender for data via sender getter method) Example: day month, year, hour minute second ==> date
  • Divergent Change - If you have a fixed class that does distinctly different things consider separating out the varying code into varying classes (extract class) that either subclassor are contained by the non-varying class.
  • Shotgun Surgery - The smell: a change in one class repeatedly requires little changes in a bunch of other classes. try to move method and move field to get all the bits into one class since they areobviously highly dependent.
  • Feature Envy - Method in one class uses lots of pieces from another class. move method to move it to the other class.
  • Data Clumps - Data that's always hanging with each other (e.g. name street zip). Extract out a class (extract class) for the data. Will help trim argument lists too since name street zip now passed as one address object.
  • Switch (case) statements - Use inheritance and polymorphism instead (exampleof this was in Fowler Chapter 1; this is one of the more difficult refactorings)
  • Lazy Class - Class doesn't seem to be doing anything. Get rid of it!
  • Speculative generality - Class designed to do something in the future but never ends up doing it. Thinking too far ahead or you though you needed this generality but you didn't. like above, collapse hierarchyor inline class
  • Message chains - Say you want to send a message toobject D in class A but you have to go through B to get C and C to get D. use hide delagate to hide C and D in B, and add a method to B that does what A wanted to do with D.
  • Inappropriate Intimacy - Directly getting in and munging with the internals of another class. To fix this, move methods, inline methods, to consolidate the intimate bits.
  • Incomplete Library Class - If method missing from library, and we can't change the library, so either: o make this method in yourobject (introduce foreign method) If there is a lot of stuff you want to change: o make yourown extension/subclass (introduce local extension)
  • Data Class - We have already talked about this extensively: in data-centric design, there are some data classes which are pretty much structs: no interesting methods. first don't letother directly get and set fields (make them private) and don't have setter for things outsiders shouldn't change look who uses the data and how they use it and move someof that code to the data class via a combinationofextract method and move method (see the Fowler chapter 1 example for several examplesof this)
  • Comments - Comments in the middleof methods are deodorant. You should really refactor so each comment block is its own method. Do extract method.

Check out the following to know more on the subject: