Refactoring

Presentation I gave at Hacker School on Refactoring.

Transcript

1. Refactoring Lee Edwards Lead Engineer, SideTour @terronk Friday, August 24, 12
2. Friday, August 24, 12
3. xkcd #844 Friday, August 24, 12
4. A History of Rewrites according to Josh Kerr • Netscape Navigator 6.0 • Microsoft Pyramid • TextMate 2 Friday, August 24, 12
5. “The single worst strategic mistake that any software company can make” - Joel Spolsky Friday, August 24, 12
6. Get good at refactoring now. Friday, August 24, 12
7. What is Refactoring? Friday, August 24, 12
8. Refactoring (noun) • A change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior. Friday, August 24, 12
9. Refactor (verb) • To restructure software by applying a series of refactorings without changing its observable behavior. Friday, August 24, 12
10. Refactor (refactored) • To restructure software by applying a series of changes to its internal structure to make it easier to understand and cheaper to modify without changing its observable behavior. Friday, August 24, 12
11. Refactoring, the Infographic Friday, August 24, 12
12. A Simple Refactoring 1 def self.amazing_cities(cities) 2 amazing_cities = [] 3 cities.each do |city| 4 if city.name == "New York" 5 amazing_cities << city 6 elsif city.rating > 5 7 amazing_cities << city 8 end 9 end 10 end 1 def self.amazing_cities(cities) 2 cities.select do |city| 3 city.name == "New York" || city.rating > 5 4 end 3 end 1 describe "amazing_cities" do 2 it "includes only cities named New York or with a rating >5" do 3 portland = City.new(name: "Portland", rating: 5) 4 nyc = City.new(name: "New York") 5 boston = City.new(name: "Boston", rating: 6) 6 amazing_cities(portland, nyc, boston).should =~ [nyc, boston] 7 end 8 end Friday, August 24, 12
13. What isn’t Refactoring? Friday, August 24, 12
14. Changes to Style 1 def amazing_cities(cities) 2 cities.select do |city| 3 city.name == "New York" || city.rating > 5 4 end 3 end 1 def amazing_cities(cities) 2 cities.select { |city| city.name == "New York" || city.rating > 5 } 3 end Friday, August 24, 12
15. Changes to Behavior 1 def amazing_cities(cities) 2 cities.select do |city| 3 city.name == "New York" || city.rating > 5 4 end 3 end 1 def amazing_cities(cities) 2 cities.select do |city| 3 (city.name == "New York" && city.state == "NY") || city.rating > 5 4 end 3 end Friday, August 24, 12
16. Why Refactor? Friday, August 24, 12
17. To increase velocity Bad code is hard to change Friday, August 24, 12
18. To make future change possible It’s harder to change code you don’t understand Friday, August 24, 12
19. To understand legacy code “Code is easier to write than to read” - Joel Spolsky Friday, August 24, 12
20. To minimize future bugs You can’t debug what you don’t understand Friday, August 24, 12
21. To raise your bus count Store knowledge in code, not somebody’s head Friday, August 24, 12
22. To make architectural decisions less binding You can always change it later! Friday, August 24, 12
23. To learn and grow Learning from past mistakes Friday, August 24, 12
24. How to Refactor 1. make sure the tests are green 2. apply refactorings 3. make sure the tests are still green 4. repeat until perfect code Friday, August 24, 12
25. When not to Refactor Friday, August 24, 12
26. When not to Refactor • Throw-away code (Lean MVP) * • Hackathons • On a tight deadline * • Rebooting a code base in a new language * Refactor later Friday, August 24, 12
27. When to Refactor Friday, August 24, 12
28. When to Refactor • After your unit tests turn green • After a working commit • When you need to understand something • The Rule of Three • When you feel pain • slow feature development • frequent bugs • When you encounter a code smell Friday, August 24, 12
29. Code Smells Friday, August 24, 12
30. Common Code Smells • Large classes • Long methods • Copy/Paste DRY • Switch/Case Statements • Ruby “try” • Code comments • Speculative generality YAGNI • Data clumps • Shotgun surgery • Inappropriate intimacy Friday, August 24, 12
31. Learning from Mistakes 1 class User < ActiveRecord::Base 2 def subscribe! 3 Gibbon.new(MAILCHIMP_KEY) 4 .list_subscribe( 5 id: GENERAL_LIST_ID, 6 email_address: email, 7 send_welcome: false, 8 double_optin: false 9 ) 10 end 11 end 1 class Host < ActiveRecord::Base 2 def subscribe! 3 Gibbon.new(MAILCHIMP_KEY) 4 .list_subscribe( 5 id: HOST_LIST_ID, 6 email_address: primary_email, 7 send_welcome: false, 8 double_optin: false 9 ) 10 end 11 end 1 class MailingList < ActiveRecord::Base 2 def subscribe!(user) 3 Gibbon.new(MAILCHIMP_KEY) 4 .list_subscribe( 5 id: HOST_LIST_ID, 6 email_address: user.email, 7 send_welcome: false, 8 double_optin: false 9 ) 10 end 11 end Friday, August 24, 12
32. Refactored! 1 class User < ActiveRecord::Base 2 def subscribe! 3 Service::Mailchimp.subscribe(email, USER_LIST_ID) 4 end 5 end 1 class Host < ActiveRecord::Base 2 def subscribe! 3 Service::Mailchimp.subscribe(primary_email, HOST_LIST_ID) 4 end 5 end 1 class MailingList < ActiveRecord::Base 2 def subscribe!(user) 3 Service::Mailchimp.subscribe(user.email, USER_LIST_ID) 4 end 5 end Friday, August 24, 12
33. DRY 1 class Service::Mailchimp 2 API = Gibbon.new(MAILCHIMP_KEY) 3 4 def self.subscribe(email, list_id, options={}) 5 API.list_subscribe(id: list_id, 6 email_address: email, 7 send_welcome: false, 8 double_optin: false, 9 merge_vars: options) 10 end 11 end Friday, August 24, 12
34. But Don’t Do This 1 class User < ActiveRecord::Base 2 include Alphabetizable 3 end 4 5 class City < ActiveRecord::Base 6 include Alphabetizable 7 end 8 9 class Movie < ActiveRecord::Base 10 scope :alphabetical, order(:title) 11 end 12 13 module Alphabetizable 14 def self.included(base) 15 base.class_eval do 16 scope :alphabetical, order(:name) 17 end 18 end 19 end 1 class User < ActiveRecord::Base 2 scope :alphabetical, order(:name) 3 end 4 5 class City < ActiveRecord::Base 6 scope :alphabetical, order(:name) 7 end 8 9 class Movie < ActiveRecord::Base 10 scope :alphabetical, order(:title) 11 end Friday, August 24, 12
35. ...Or This 1 class User 2 [:accounts, :orders, :friends, :credits, :events, :tickets].each do |association| 3 class_eval do 4 has_many association 5 end 6 end 7 end 1 class User 2 has_many :accounts 3 has_many :orders 4 has_many :friends 5 has_many :credits 6 has_many :events 7 has_many :tickets 8 end Friday, August 24, 12
36. Refactoring Tools • Test Suite • Compiler • Many IDEs • Code Climate Friday, August 24, 12
37. Refactoring Blog Posts • “Textmate 2 and Why You Shouldn’t Rewrite Your Code” by Josh Kerr • “Things You Should Never Do” by Joel Spolsky • “The Big Rewrite” by Chad Fowler Friday, August 24, 12
38. Refactoring Books Friday, August 24, 12