Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Refactoring Code Smells into Clean Code

Refactoring Code Smells into Clean Code

I prepared a small deck with lots of information having references to Martin Fowler's Refactoring book (version 1).

Lemi Orhan Ergin

December 13, 2017
Tweet

More Decks by Lemi Orhan Ergin

Other Decks in Programming

Transcript

  1. Refactoring (verb) To restructure so!ware by applying a series of

    refactorings without changing its observable behavior. — Martin Fowler, Refactoring: Improving the Design of Existing Code
  2. Coding Automated Testing Refactoring Writing production code Writing test code

    & validating production code and design Minimizing complexity Communicating Using feedback loops via tools and people DNA PROGRAMMING OF
  3. Scaling-up includes the use of periodic restructuring, just enough design,

    and evolutionary architecture. Identifies design-maintenance issues (“code smells”) that typically represent violations of known principles of good design. Applies incrementally and iteratively a set of design improvement techniques (“refactorings”). Minimize complexity & duplication in order to maximize simplicity & ease-of-change is the goal. Encourages the “right” design details to emerge “just-in-time” with minimal guesswork/rework. REFACTORING
  4. REFACTORING IS NOT “Rework” – redesigning things that could, and

    should, have been designed correctly in the first place.
  5. REFACTORING IS NOT Design “gold-plating” – work that adds no

    business value, and merely serves to stroke the egos of perfectionists who are out of touch with business reality.
  6. REFACTORING IS NOT Miscellaneous code “tidying” – the kind that

    is “nice to have,” but should only happen when the team has some slack-time, and is a luxury we can do without, without any serious consequences.
  7. REFACTORING IS NOT A license to “hack” – avoiding any

    and all initial design & analysis and instead jumping straight to coding with no “real” design.
  8. REFACTORING IS NOT Reengineering – large-scale restructuring that requires a

    concerted effort over the course of several weeks/months to re-write or re-architect significant parts of the system.
  9. While developing so!ware, you divide your time between two distinct

    activities: adding function and refactoring. When you add function, you shouldn’t be changing existing code; you are just adding new capabilities. You can measure your progress by adding tests and ge#ing the tests to work. When you refactor, you make a point of not adding function; you only restructure the code. You don’t add any tests; you only change tests when you absolutely need to in order to cope with a change in an interface. Martin Fowler, Refactoring while talking about "two hats metaphor" of Kent Beck
  10. STEPS FOR REFACTORING the removal of duplication & dependencies the

    simplification of complex logic/structure the clarification of unclear code & design intentions 1 2 3
  11. Each step is small and simple, even simplistic. You move

    a field from one class to another, pull some code out of a method to make into its own method, and push some code up or down a hierarchy. To refactor safely, you must run tests to ensure your changes didn't break anything. You will have more confidence to refactor and make other design changes if you can quickly run automated tests. Refactoring in small steps helps prevent introducing defects. Most refactorings take only seconds or minutes to perform. Even large restructurings are implemented in small steps. MAKE IT SMALL LET IT GROW WITH TESTS
  12. RANDOM REFACTORING BY CLASS RENAMING Select a random class If

    it is not an entity or a value object Add intention to the name if not exists
 Extract new structures into new classes or methods Go to line 1 1 2 3 4 5 That turns our big fat X_Manager into A_Remover, B_Provider, C_Calculator, D_Creator, E_Validator. Now we can start to talk about Single Responsibility Rule. Developers hate creating classes with few lines. Even though the majority know big fat classes need refactoring, they o!en prefer few number of big fat classes over many small classes. Here is a way of breaking this habit.
  13. In my view, refactoring is not an activity you set

    aside time to do separately from implementation activities. Refactoring is something you do all the time in li#le bursts. Martin Fowler, Refactoring: Improving the Design of Existing Code
  14. When touching any code, leave the codebase cleaner than you

    found it! “Uncle” Bob Martin’s The Boy Scout Rule
  15. WHEN TO REFACTOR While adding functionality While fixing a bug

    While reviewing code A!er coding the same/similar thing for the third time A!er the third time you deferred refactoring a change, 
 for any reason Before the end of the iteration if you haven’t been 
 following The Rule of Three 1 2 3 4 5 6 The Rule of Three The Rule of Three The Rule of Three Martin Fowler, Refactoring: Improving the Design of Existing Code ?
  16. Emergent Design is a fancy name for the resulting design

    that “emerges” from the synergy of combining Refactoring together with TDD, Continuous Integration and Automated Testing. Design, rather than occurring all up front, occurs continuously during development. The cumulative effect of these small changes can radically improve the design. EMERGENT DESIGN
  17. ARCHITECTURE In most successful so!ware projects, the expert developers working

    on that project have a shared understanding of the system design. This shared understanding is called "architecture." This understanding includes how the system is divided into components and how the components interact through interfaces. Ralph Johnson
  18. ARCHITECTURE Architecture is about important stuff. Whatever that is. Stuff

    that’s hard to change. There should be as li#le of that stuff as possible. Martin Fowler, in a conversation
  19. So!ware Architecture concerns infrastructure elements that must exist before you

    can begin execution. Architecture is about things that are hard to change later, it is difficult to allow an architecture to emerge. However, just because we can't allow architecture to emerge doesn't mean that it can't evolve. If we create an initial, flexible architecture and take special care to not create an irreversible decision, then we can allow it to evolve over time as new concerns appear. — Neal Ford, Evolutionary Architecture and Emergent Design Key techniques of Evolutionary Architecture include: Deferring Irreversible Decisions to the “Last Responsible Moment” (LRM Principle) Architectural “Spike” (a.k.a. Architectural “Deep Dive”) Architecture Iteration and/or Spike Iteration EVOLUTIONARY ARCHITECTURE
  20. Long Method Large Class Primitive Obsession Long Parameter List Variable

    Clumps BLOATERS Bloaters are code, methods and classes that have huge amount of content inside
  21. Switch Statements Temporary Field Refused Bequest Identical Functions with Different

    Interfaces OBJECT ORIENTATION ABUSERS All these smells are incomplete or incorrect application of object-oriented programming principles.
  22. CHANGE PREVENTERS These smells mean that if you need to

    change something in one place in your code, you have to make many changes in other places too. Divergent Change Shotgun Surgery Parallel Inheritance Hierarchies
  23. DISPENSABLES A dispensable is something pointless and unneeded whose absence

    would make the code cleaner, more efficient and easier to understand. Comments Duplicate Code Loser Class Pale Data Class Dead Code Speculative Generality
  24. COUPLERS All the smells in this group contribute to excessive

    coupling between classes or show what happens if coupling is replaced by excessive delegation. Feature Envy Inappropriate Intimacy Message Chains Middle Man Incomplete Library Class
  25. Extract Method void printOwing() { printBanner(); //print details System.out.println("name: "

    + name); System.out.println("amount: " + getOutstanding()); } void printOwing() { printBanner(); printDetails(getOutstanding()); } void printDetails(double outstanding) { System.out.println("name: " + name); System.out.println("amount: " + outstanding); } You have a code fragment that can be grouped together. Move this code to a separate new method (or function) and replace the old code with a call to the method. 1
  26. int getRating() {
 return (moreThanFiveLateDeliveries()) ? 2 : 1; }

    
 boolean moreThanFiveLateDeliveries() { return _numberOfLateDeliveries > 5; } 2
  27. Inline Method int getRating() {
 return (moreThanFiveLateDeliveries()) ? 2 :

    1; } 
 boolean moreThanFiveLateDeliveries() { return _numberOfLateDeliveries > 5; } int getRating() {
 return (_numberOfLateDeliveries > 5) ? 2 : 1; } Someone is using too much indirection and it seems that every method does simple delegation to another method, and I get lost in all the delegation You inline the various calls made by the method that have behavior you want to have in the method object. It’s easier to move one method than to move the method and its called methods. 2
  28. 3

  29. Introduce Parameter Object Your methods contain a repeating group of

    parameters. Replace these parameters with an object. 3
  30. Preserve Whole Object You get several values from an object

    and then pass them as parameters to a method. Instead, try passing the whole object. int low = daysTempRange().getLow(); int high = daysTempRange().getHigh(); boolean withinPlan = plan.withinRange(low, high); boolean withinPlan = plan.withinRange(daysTempRange()); 4
  31. class Order { //... public double price() { double primaryBasePrice;

    double secondaryBasePrice; double tertiaryBasePrice; // long computation. //... } } 5
  32. Replace Method with Method Object class Order { //... public

    double price() { double primaryBasePrice; double secondaryBasePrice; double tertiaryBasePrice; // long computation. //... } } class Order { //... public double price() { return new PriceCalculator(this).compute(); } } class PriceCalculator { private double primaryBasePrice; private double secondaryBasePrice; private double tertiaryBasePrice; public PriceCalculator(Order order) { // copy relevant information from order object. //... } public double compute() { // long computation. //... } } You have a long method in which the local variables are so intertwined that you cannot apply Extract Method. Transform the method into a separate class so that the local variables become fields of the class. Then you can split the method into several methods within the same class. 5
  33. if (date.before(SUMMER_START) || date.after(SUMMER_END)) { charge = quantity * winterRate

    + winterServiceCharge; } else { charge = quantity * summerRate; } 6
  34. Decompose Conditional You have a complex conditional (if-then/else or switch).

    Decompose the complicated parts of the conditional into separate methods: the condition, then and else. if (date.before(SUMMER_START) || date.after(SUMMER_END)) { charge = quantity * winterRate + winterServiceCharge; } else { charge = quantity * summerRate; } if (notSummer(date)) { charge = winterCharge(quantity); } else { charge = summerCharge(quantity); } 6
  35. 7

  36. Extract Class When one class does the work of two,

    awkwardness results. Instead, create a new class and place the fields and methods responsible for the relevant functionality in it. 7
  37. Extract Sub-Class A class has features that are used only

    in certain cases. Create a subclass and use it in these cases. 8
  38. A class (or group of classes) contains a data field.

    The field has its own behavior and associated data. 9
  39. Replace Data Value with Object A class (or group of

    classes) contains a data field. The field has its own behavior and associated data. Create a new class, place the old field and its behavior in the class, and store the object of the class in the original class. 9
  40. 10

  41. Replace Type Code with Class A class has a field

    that contains type code. The values of this type are not used in operator conditions and do not affect the behavior of the program. Create a new class and use its objects instead of the type code values. 10
  42. 11

  43. Replace Type Code with Subclasses You have a coded type

    that directly affects program behavior (values of this field trigger various code in conditionals). Create subclasses for each value of the coded type. Then extract the relevant behaviors from the original class to these subclasses. Replace the control flow code with polymorphism. 11
  44. public class Notebook { private int status; // 0=PRODUCED, 1=SOLD

    private String model; // XSI-Q2TU-17-1 public String getSeries() { return this.model.split("-")[0]; } public String getModelYear() { return this.model.split("-")[2]; } } 12
  45. Remove Invisible Logic from Data public class Notebook { private

    int status; // 0=PRODUCED, 1=SOLD private String model; // XSI-Q2TU-17-1 public String getSeries() { return this.model.split("-")[0]; } public String getModelYear() { return this.model.split("-")[2]; } } public class NotebookModel { final String series; final String subseries; public NotebookModel(String series, String subseries) { this.series = series; this.subseries = subseries; } } public enum NotebookStatus { PRODUCED, SOLD; } public class Notebook { long id; NotebookStatus status; NotebookModel model; } You keep data and assign a logic behind the scenes. The best way to remove the logic on data is not to keep it at all. 12
  46. int basePrice = quantity * itemPrice; double seasonDiscount = store.getSeasonalDiscount();

    double fees = store.getFees(); double finalPrice = discountedPrice(basePrice, seasonDiscount, fees); 13
  47. Replace Parameter with Method Call int basePrice = quantity *

    itemPrice; double seasonDiscount = store.getSeasonalDiscount(); double fees = store.getFees(); double finalPrice = discountedPrice(basePrice, seasonDiscount, fees); int basePrice = quantity * itemPrice; double finalPrice = discountedPrice(basePrice, store); Before a method call, a second method is run and its result is sent back to the first method as an argument. But the parameter value could have been obtained inside the method being called. Instead of passing the value through a parameter, place the value-ge#ing code inside the method. 13
  48. Move Method A method is used more in another class

    than in its own class. Create a new method in the class that uses the method the most, then move code from the old method to there. Turn the code of the original method into a reference to the new method in the other class or else remove it entirely. 14
  49. class Bird { //... double getSpeed() { switch (type) {

    case EUROPEAN: return getBaseSpeed(); case AFRICAN: return getBaseSpeed() - getLoadFactor() * numberOfCoconuts; case NORWEGIAN_BLUE: return (isNailed) ? 0 : getBaseSpeed(voltage); } throw new RuntimeException("Should be unreachable"); } } 15
  50. Replace Conditional with Polymorphism abstract class Bird { //... abstract

    double getSpeed(); } class European extends Bird { double getSpeed() { return getBaseSpeed(); } } class African extends Bird { double getSpeed() { return getBaseSpeed() - getLoadFactor() * numberOfCoconuts; } } class NorwegianBlue extends Bird { double getSpeed() { return (isNailed) ? 0 : getBaseSpeed(voltage); } } // Somewhere in client code speed = bird.getSpeed(); Create subclasses matching the branches of the conditional. In them, create a shared method and move code from the corresponding branch of the conditional to it. Then replace the conditional with the relevant method call. The result is that the proper implementation will be a#ained via polymorphism depending on the object class. 15
  51. void setValue(String name, int value) { if (name.equals("height")) { height

    = value; return; } if (name.equals("width")) { width = value; return; } Assert.shouldNeverReachHere(); } 16
  52. Replace Parameter with Explicit Methods void setValue(String name, int value)

    { if (name.equals("height")) { height = value; return; } if (name.equals("width")) { width = value; return; } Assert.shouldNeverReachHere(); } void setHeight(int arg) { height = arg; } void setWidth(int arg) { width = arg; } A method is split into parts, each of which is run depending on the value of a parameter. Extract the individual parts of the method into their own methods and call them instead of the original method. 16
  53. Introduce Null Object if (customer == null) { plan =

    BillingPlan.basic(); } else { plan = customer.getPlan(); } class NullCustomer extends Customer { boolean isNull() { return true; } Plan getPlan() { return new NullPlan(); } // Some other NULL functionality. } // Replace null values with Null-object. customer = (order.customer != null) ? order.customer : new NullCustomer(); // Use Null-object as if it's normal subclass. plan = customer.getPlan(); Since some methods return null instead of real objects, you have many checks for null in your code. Instead of null, return a null object that exhibits the default behavior. 17
  54. You have a subclass that uses only a portion of

    the methods of its superclass (or it's not possible to inherit superclass data). 18
  55. Replace Inheritance with Delegation You have a subclass that uses

    only a portion of the methods of its superclass (or it's not possible to inherit superclass data). Create a field and put a superclass object in it, delegate methods to the superclass object, and get rid of inheritance. 18
  56. 19

  57. Extract Superclass You have two classes with common fields and

    methods Create a shared superclass for them and move all the identical fields and methods to it. 19
  58. You have an expression that is hard to understand. void

    renderBanner() { if ((platform.toUpperCase().indexOf("MAC") > -1) && (browser.toUpperCase().indexOf("IE") > -1) && wasInitialized() && resize > 0 ) { // do something } } 20
  59. Extract Variable You have an expression that is hard to

    understand. Place the result of the expression or its parts in separate variables that are self-explanatory. void renderBanner() { if ((platform.toUpperCase().indexOf("MAC") > -1) && (browser.toUpperCase().indexOf("IE") > -1) && wasInitialized() && resize > 0 ) { // do something } } void renderBanner() { final boolean isMacOs = platform.toUpperCase().indexOf("MAC") > -1; final boolean isIE = browser.toUpperCase().indexOf("IE") > -1; final boolean wasResized = resize > 0; if (isMacOs && isIE && wasInitialized() && wasResized) { // do something } } 20
  60. The client gets object B from a field or method

    of object А. Then the client calls a method of object B. 21
  61. Hide Delegate The client gets object B from a field

    or method of object А. Then the client calls a method of object B. Create a new method in class A that delegates the call to object B. Now the client does not know about, or depend on, class B. 21
  62. Remove Middle Man A class has too many methods that

    simply delegate to other objects. Delete these methods and force the client to call the end methods directly. 22
  63. A utility class does not contain the method that you

    need and you cannot add the method to the class. class Report { //... void sendReport() { Date nextDay = new Date(previousEnd.getYear(), previousEnd.getMonth(), previousEnd.getDate() + 1); //... } } 23
  64. A utility class does not contain the method that you

    need and you cannot add the method to the class. Introduce Foreign Method class Report { //... void sendReport() { Date nextDay = new Date(previousEnd.getYear(), previousEnd.getMonth(), previousEnd.getDate() + 1); //... } } class Report { //... void sendReport() { Date newStart = nextDay(previousEnd); //... } private static Date nextDay(Date arg) { return new Date(arg.getYear(), arg.getMonth(), arg.getDate() + 1); } } Add the method to a client class and pass an object of the utility class to it as an argument. 23
  65. Introduce Local Extension A utility class does not contain some

    methods that you need. But you cannot add these methods to the class. Create a new class containing the methods and make it either the child or wrapper of the utility class. 24
  66. Make smaller things, and let them know very very little

    from each other. Small methods are simpler.
  67. Write tests if you want to refactor. Keep tests green,

    then refactor Refactor to the point you delete 
 the code you don’t understand. REFACTOR WITH TESTS
  68. Make the change easy first, then make the easy change

    Kent Beck önce degisiklikleri basitlestirmeye çalıs
 sonra en basit degisikligi yapabilirsin