Refactoring

1. Compl Refactoring

2. “Bad smells” in your software Code duplication Long operations, big classes Operations with lots of parameters Divergent or shotgun fixes during maintenance Feature envy You use “switch” (*) “fake attributes” in your class comments inside an operation (Eh? I’m sorry!?)

3. Suboptimaal software • Mitigated by: invest in a good design, employ design patterns • Still, under pressure (deadline!) the temptation to do dirty solving is big... • Let’s take a look at “refactoring” Complexity & flexibility of your software components are not optimal. maintenance cost  bugs fix, adapting features, adding featuresDon’t underestimate maintenance.

4. Refactoring Refactoring is a transformation on the source code of a software to make it easier to understand and to maintain. The transformation must preserve the software’s observable behavior. Belong to basic vocabof sw. engineers. So, performance is not a consideration in refactoring. This is not to say that performance is unimportant. But “refactoring” is limited to achieve the above mentioned goals. Fowler, 2000. With contrib. from e.g. Erich Gamma (GOF) & Kent Beck (JUnit)

5. How to do it? Keep in mind that the goal of refactoring is not a cosmetic make-over, though some of the steps may look cosmetics. The goal is: mitigating your long term maintenance. I will mark the refactoring names like this: Extract Method Formal theory of refactoring?  sadly none practical enough. So, we do it in a series of small steps, like ... Rely on testing (!) Intergrate it structurally into your maintenance workflow

6. Example • RentInfo • numOfDays • Film • title • priceCode: int • Customer • name • CustPoints • + rent(film) • + back(film) • + mkInvoice() * * film 1 We’ll keep getter and setter implicit. film rentInfos Three price categories, encoded as constants: CHILDREN= 0 STANDARD= 1 NEW = 2 * * • RentInfo • numOfDays F1 : Film 3d : RentInfo C: Customer 7d : RentInfo F2 : Film

7. mkInvoice () { totPrice = 0 pts = 0 println(“Invoice for” + name) for(RentInfov : rentInfos) { // calculate price for a single film v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = 3 euro * v.numOfdays ; break case Film.NEW : v_price = 5 euro * v.numOfdays ; break case Film.CHILDREN : v_price = 2 euro * v.numOfdays } totPrice += v_price // calculate earned points pts++ if (v.getFilm().getPriceCode()==Film.NEW) pts++ println(“Film ” + v.getName() + “, price = ” + v_price) } println(“Tot price = ” + totPrice) println(“Points earned = ” + pts) } Bad smells? Let’s rework this, but in small steps  refactoring.

8. Extract method promote a fraction of code to a method. (commentsoftengive hint) mkInvoice () { totPrice = 0 pts = 0 println(“Invoice for” + name) for(RentInfov : rentInfos) { // calculate price for a single film v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = ... ; break case Film.NEW : v_price = ... ; break case Film.CHILDREN : v_price = ... } totPrice += v_price // calculate earned points pts++ if (v.getFilm().getPriceCode()==Film.NEW) pts++ println(“Film ” + v.getName() + “, price = ” + v_price) } println(“Tot price = ” + totPrice) println(“Points earned = ” + pts) }

9. calculatePrice(v) { v_price= 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = ... ; break case Film.NEW : v_price = ... ; break case Film.CHILDREN : v_price = ... } return v_price } Extract method mkInvoice () { totPrice = 0 pts = 0 println(“Invoice for” + name) for(RentInfov : rentInfos) { // calculate price for a single film v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = ... ; break case Film.NEW : v_price = ... ; break case Film.CHILDREN : v_price = ... } totPrice += v_price // calculate earned points pts++ if (v.getFilm().getPriceCode()==Film.NEW) pts++ println(“Film ” + v.getName() + “, price = ” + v_price) } println(“Tot price = ” + totPrice) println(“Points earned = ” + pts) } v_price = calculatePrice(v)

10. Extract method mkInvoice () { totPrice = 0 pts = 0 println(“Invoice for” + name) for(RentInfov : rentInfos) { // calculate price for a single film .... totPrice += v_price // calculate earned points pts++ if (v.getFilm().getPriceCode()==Film.NEW) pts++ println(“Film ” + v.getName() + “, price = ” + v_price) } println(“Tot price = ” + totPrice) println(“Points earned = ” + pts) } Take sufficient context along ?? pts = calculateEarnedPoints(v,pts) caculateEarnedPoints(v,pts) { pts++ if (v.getFilm().getPriceCode()==Film.NEW) pts++ return pts }

11. After extraction mkInvoice () { totPrice = 0 pts = 0 println(“Invoice for” + name) for(RentInfov : rentInfos) { v_price = calculatePrice(v) totPrice += v_price pts = calculateEarnedPoints(v,pts) println(“Film ” + v.getName() + “, price = ” + v_price) } println(“Tot price = ” + totPrice) println(“Points earned = ” + pts) } these were comments! Now self-documenting. This is easier to understand now, no?

12. Don’t we all love temporary variables? intp= calculatePrice(v) totPrice += p println(“Film ” + v.getName() + “, price = ” + p) Conceptually, v_price is just use to hold the result of calculatePrice. They are synonymous; but we have introduced it to get a bit of performance. But it does complicate your code! (your mind have to keep track of one more item in your program) Remove temp totPrice += calculatePrice(v) println(“Film ” + v.getName() + “, price = ” + calculatePrice(v)) • How about performance? • Balancing between maintenance and performance. • 90% of the time, your software is doing just 10% of your code. • User profiler to locate this 10%, and target your performance optimization on this 10%.

13. “Features envy” Film calculatePrice(v) Customer calculatePrice(v) calculatePrice(v) { v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = 3 euro * v.numOfdays ; break case Film.NEW : v_price = 5 euro * v.numOfdays ; break case Film.CHILDREN : v_price = 2 euro * v.numOfdays } return v_price } Customer is doing too much work on data obtained from Film. Shouldn’t Film does this work instead??  risk of shot-gun fixes in the future  “Move method”

14. Why is this switch bad? Film calculatePrice()v Film calculatePrice(v) calculatePrice(v) { v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = ... ; break case Film.NEW : v_price = ... ; break case Film.CHILDREN : v_price = ... } return v_price } StandardFilm NewFilm ChildrenFilm Solving it with inheritence. Unfortunaly in this example, “film-type” should be able to change dynamically! So you need a different solution. Adding a new kind of film forces you to change calculatePrice divergent fix.

15. Replace type-code with strategy + polymorphism priceStrg Film calculatePrice(v) 1 FilmPriceStrategy calculatePrice(v) calculatePrice() { v_price = 0 switch (v.getFilm().getPriceCode()) { case Film.STANDARD : v_price = ... ; break case Film.NEW : v_price = ... ; break case Film.CHILDREN : v_price = ... } return v_price } StandardPrice NewPrice ChildrenPrice You can change the strategy dynamically: film.priceStrg = new NewPrice() film.priceStrg = new StandardPrice() calculatePrice(v) { return priceStrg.calculatePrice(v) }

16. Fowler’s refactoring set Streamlining methodes (9) Moving features (8) Organizing data (16) Simplifying conditional statements (8) Simplifying method calls (15) Generalization (12)

17. Streamlining methods Fowler: long methods are often the sources of problems (he prefers more methods; but short) • extract method • inverse: inline method but you’ll risk code duplication. To limit: apply if the body is just as clear as the method’s name, and is not called in too many other methods. • eliminate and introduce temp • temp is a form of indirection • furthermore makes method extraction more difficult as you have to move its context as well

18. Temp p =product.getPrice() * 1.5 > 10.0 expression is too complex double sellPrice= product.getPrice() * 1.5 p = sellPrice > 10.0 p = sellPrice() > 10.0 introduce explaining variable and eliminatetemp replace temp with query you can’t elimimate all temps  loop counter, accumulation var.

19. You have too many parameters … price(date , age, isMember, discountCard, isOpenDay) personProfile age isMember discountCard price(personProfile, dateProfile) dateProfile date isOpenday Introduce parameter object

20. Moving Features Distribute the work (which class does what). A class that does too much is difficult to understand. Main tool: move method/field (you have seen this) spliting and combining classes introducing and eliminating delegation

21. Split and combine Person name age street housenr postcode … printAddress() Person name age ... 1 1 Address street housenr postcode printAddress() Extract class: a class C does too much. Identify a part of C that can be group to its own class. inverse: inline class.

22. Delegate Person address. getCity() 1 Postcode getCity() Address getCity() A client (Person) of Address is calling Address’ delegate  Hide this delegate. 1 Person getCity() { return postcode.getCity() } address.getPostcode().getCity() If Address is doing too mush delegation work instead of real work, then reverse: let the client access the delagate directly. 1 Postcode getCity() Address 1 Hide delegate remove middle man

23. Streamlining conditionals Complex domain logics translate to complex conditional statements, which are often difficult to understand (hence error prone). • Main tool : extract method on conditionals • decompose conditional, consolidate conditional, • Simplification on the structure • remove control flag • replace conditional with polymorphism (you have seen this) • replace nested conditional with guard clauses

24. Extract method on conditions A complicate conditional: if(date.before(SUMMER_BEGIN) || date.after(SUMMER_END) ) price = 0.8 * price + ... // some complicated expr Decompose conditional if (notSummer(date ) ) price = 0.8 * price ... if (notSummer(date ) ) price = summerPrice(...)

25. Extract method on conditions Series of “if” encoding “or” Cascade of “if” encoding “and” giveCandy(person) { if (person.age ≤ 10) return candy if (person.isMember) return candy if (person.isBirthday()) return candy return null } price(person) { if (person.age ≤ 10) if (person.isMember) if (person.isBirthday()) return 0 ; return stdPrice(person) } Consolidate conditional expression price(person) { if (isFree(person)) return0 ; returnstdPrice(person) } giveCandy(person) { if (candyEligible(person)) return candy ; return null }

26. Simplifying cond. structure control variable found = false product = null for (iter = V.iterator() ; iter.hasNext() && !found ; ) { product = iter.next() found = product.price() <= 10.0 } return product remove control flag if (product.prijs() <= 10.0) break for (Product p : V) { product = p if (product.prijs() <= 10.0) break } return product

27. Simplifying cond. structure “If” implementing two paths of your “normal flow” : giveCandy(person) { if (person.age < 12) c = new MagicCandy() else c = new StdCandy() c.wrap() return c } An “alternate flow” seeks to break off the computation early, due to some “exceptional” condition. “If” implementing an “alternate flow” : giveMilk(person) { if (person is alergic) m = null else { m = new Milk() ; m.warmup() } return m }

28. Simplifying cond. structure An “alternate flow” break off the computation early, due to some “exceptional” condition. “If” implementing an “alternate flow” : giveMilk(person) { if (person is alergic) m = null else { m = new Milk() ; m.warmup() } return m } Here programmer also tried to make the program to have a single entry and exit point. That seems to be a good idea, no? Fowler: favor whichever is more readable. Replace (nested) conditionals with “guard clauses” giveMilk(person) { if (person is alergic) return null m = new Milk() ; m.warmup() return m }

29. Generalization Inheritence is importtant in OO; optimize your inheritence structure. Moving features within a hierarchy Spliting and combining hierarchy Replacing inheritence with delegation, or the other way around

30. Pull up / pull down Product name discount() Product name pull up Apple discount() Coffee discount() Apple Coffee Product name drink() Product name pull down Coffee drink() Apple Apple Coffee GreenApple collapsehierarchy GreenApple VeryGreenApple

31. Extract sub/super class Product name extract subclass Product name discount() DiscountedProduct discount() But discount is only relevant for a subset of the instances of Product. ItemWithIcon getIcon () setIcon(i) resetIcon() Product price getIcon () setIcon(i) resetIcon() extract superclass Product price The classes have a common subset of features Person name getIcon () setIcon(i) resetIcon() Person name (Q: and what if we don’t have multiple inheritance?)

32. Extract interface WebShop Invoice we have multiple client-classes that use the same subset of Product’s features WebShop Invoice <<interface>> SellableProduct getPrice() getTax() getDiscount() extract interface <<use>> <<use>> Product name getPrice() getTax() getDiscount() … Product name …

33. Replace delegation with inheritence, or back ItemWithIcon getIcon () setIcon(i) resetIcon() … Product name price If Product turns out to use only very few features of ItemWithIcon Replace Inheritence with delegation Replace delegation with Inheritence ItemWithIcon getIcon () setIcon(i) resetIcon() Product name price 1 0..1 If Product turns out to delegate too much work to ItemWithIcon

34. Tools • Refactoring can’t be fully automated • some refactoring steps are difficult to automate • some extent supported by IDEs, • Eclipse, Netbeans, IntelliJ • no correctness theory that is also practical enough  you will have to rely on testing