Down the Rabbit Hole: An Adventure in Legacy Code

Legacy code is unpredictable & difficult to understand. Most of us will work with legacy code, so it’s important for us to understand its major pain points & their underlying causes. You’ ll come away from this talk with practical strategies to help you understand, improve, & even love legacy code.

Transcript

1. Down the Rabbit Hole: An Adventure in Legacy Code Loren Crawford RailsConf 2018
2. Loren Crawford Software Engineer at MailChimp refactoring & testing dog named Luna @lcrawfish
3. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
4. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
5. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
6. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
7. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
8. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
9. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
10. What’s up with legacy code?
11. what’s up with legacy code?
12. “When someone makes fun of legacy code remember legacy is a funny way to spell successful. Your startup with perfect code with fail.” Dustin Shields-Cloues what’s up with legacy code?
13. legacy code is a system of working software written by many developers over time that is often hard to change & contains business logic complexities that are not easily distilled from the code* * my working definition what’s up with legacy code?
14. to change || not to change what’s up with legacy code?
15. change behavior improve design what’s up with legacy code?
16. improve design as you change behavior what’s up with legacy code?
17. Let’s go down the rabbit hole
18. add a feature
19. Yarn Hearts Online Yarn Marketplace add a feature adding a feature
20. requirements add a feature adding a feature
21. Users want to be able to search online sellers for Alpaca yarn. add a feature adding a feature
22. Alpaca yarn must also be fair trade & organic. add a feature adding a feature
23. what do you do? adding a feature
24. look at tests adding a feature
25. tests/ adding a feature
26. controllers/ helpers/ mailers/ system/ fixtures/ integration/ models/ adding a feature
27. where do I test the thing? - What class am I calling in the test? - What type of object am I using? - What is the expected behavior? adding a feature
28. reach out to your teammates adding a feature
29. let’s assume there are no tests adding a feature
30. “it’s better to confront the beast than to hide from it.” Michael Feathers ⃟ Working Effectively with Legacy Code adding a feature
31. add tests to preserve current behavior adding a feature
32. why test? adding a feature
33. Prove code works & your assumptions are correct. adding a feature
34. Prove code works & your assumptions are correct. Uncover trivial bugs. adding a feature
35. Prove code works & your assumptions are correct. Uncover trivial bugs. Document how code actually works. adding a feature
36. Prove code works & your assumptions are correct. Uncover trivial bugs. Document how code actually works. Help design better code. adding a feature
37. don’t have time? adding a feature
38. “The hurrier I go, the behinder I get.” adding a feature Lewis Carroll
39. Working Effectively with Legacy Code Michael Feathers Chapter 6 I Don’t Have Much Time and I Have to Change It adding a feature
40. example time adding a feature
41. Users want to be able to search online sellers for Alpaca yarn. add a feature adding a feature Must also be fair trade & organic.
42. Yarn add a feature adding a feature
43. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
44. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
45. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
46. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
47. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
48. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
49. adding a feature id yarn_name gauge weight color is_organic is_fair_trade seller_id … … Yarns Table
50. Seller add a feature adding a feature
51. adding a feature id name address in_good_standing is_organic is_fair_trade … … Sellers Table
52. adding a feature id name address in_good_standing is_organic is_fair_trade … … Sellers Table
53. adding a feature id name address in_good_standing is_organic is_fair_trade … … Sellers Table
54. adding a feature id name address in_good_standing is_organic is_fair_trade … … Sellers Table
55. adding a feature id name address in_good_standing is_organic is_fair_trade … … Sellers Table
56. get_sellers(...) adding a feature Yarn Model
57. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: "wool"}) found_sellers << sellers end found_sellers.flatten! preferred_sellers = [] mid_sellers = [] found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers elsif return found_sellers end adding a feature Don’t try to read this
58. when "cotton" ... when "silk" ... when "mohair" ... when "cashmere" ... when "synthetic" ... when "angora" ... else ... adding a feature
59. add tests to preserve current behavior adding a feature
60. how? add a feature adding a feature
61. let’s look at the logic around ‘wool’ add a feature adding a feature
62. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
63. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
64. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
65. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
66. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
67. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
68. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
69. add a feature adding a feature write a test for the simplest thing to test
70. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
71. RSpec.describe Yarn, "#get_seller" do context "when you pass in wool as yarn_name" do it "returns one found seller" do ... end end end adding a feature Test
72. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true) ... end adding a feature Test
73. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true, is_fair_trade: true) ... end adding a feature Test
74. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true, is_fair_trade: true, in_good_standing: true) ... end adding a feature Test
75. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true, is_fair_trade: true, in_good_standing: true) yarn = Yarn.create(name: "wool", seller: seller) ... end adding a feature Test
76. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true, is_fair_trade: true, in_good_standing: true) yarn = Yarn.create(name: "wool", seller: seller, is_organic:true) ... end adding a feature Test
77. it "returns one found seller" do seller = Seller.create(name: "Get Stitching", is_organic: true, is_fair_trade: true, in_good_standing: true) yarn = Yarn.create(name: "wool", seller: seller, is_organic:true, is_fair_trade: true) ... end adding a feature Test
78. it "returns two found sellers" do ... found_sellers = Yarn.get_sellers("wool") ... end adding a feature Test
79. it "returns two found sellers" do ... found_sellers = Yarn.get_sellers("wool") expect(found_sellers.count).to eq 1 expect(found_sellers.first).to eq(seller) end adding a feature Test
80. passes! add a feature adding a feature
81. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
82. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
83. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers else return found_sellers end adding a feature
84. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers before mid_sellers" do ... end end adding a feature Test
85. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers and mid_sellers" do preferred_seller_1 = Seller.create(name: "Preferred seller 1", is_organic: true, is_fair_trade: true) preferred_seller_2 = Seller.create(name: "Preferred seller 2", is_organic: true, is_fair_trade: true) mid_seller = Seller.create(name: "mid seller", is_organic: true, is_fair_trade: false) end end adding a feature Test
86. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers and mid_sellers" do preferred_seller_1 = Seller.create(name: "Preferred seller 1", is_organic: true, is_fair_trade: true) preferred_seller_2 = Seller.create(name: "Preferred seller 2", is_organic: true, is_fair_trade: true) mid_seller = Seller.create(name: "mid seller", is_organic: true, is_fair_trade: false) end end adding a feature Test
87. it "returns an array with preferred sellers and mid_sellers" do preferred_seller_1 = Seller.create(name: "Preferred seller 1", is_organic: true, is_fair_trade: true) preferred_seller_2 = Seller.create(name: "Preferred seller 2", is_organic: true, is_fair_trade: true) mid_seller = Seller.create(name: "mid seller", is_organic: true, is_fair_trade: false) end end adding a feature Test
88. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers and mid_sellers" do preferred_seller_1 = ... preferred_seller_2 = ... mid_seller = ... Yarn.create(name: "wool", seller: preferred_seller_1, ...) Yarn.create(name: "wool", seller: preferred_seller_2, ...) Yarn.create(name: "wool", seller: mid_seller, ...) end end adding a feature Test
89. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers and mid_sellers" do // setup found_sellers = Yarn.get_sellers("wool") end end adding a feature Test
90. context "when you have more preferred sellers than mid_sellers" do it "returns an array with preferred sellers and mid_sellers" do // setup found_sellers = Yarn.get_sellers("wool") expect(found_sellers.count).to eq 3 expect(found_sellers.last).to eq(mid_seller) end end adding a feature Test
91. passes! add a feature adding a feature
92. why did we do this? add a feature adding a feature
93. legacy code add a feature adding a feature
94. “Whenever I do a refactoring, the first step is always the same. I need to build a solid set of tests for that section of code. The tests are essential because even though I following refactorings structured to avoid most of the opportunities for introducing bugs, I’m still human and still make mistakes.” Martin Fowler Refactoring: Improving the Design of Existing Code
95. Users want to be able to search online sellers for Alpaca yarn. add a feature adding a feature
96. Sellers’ alpaca yarn must be fair trade & organic. add a feature adding a feature
97. let’s add a test add a feature adding a feature
98. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do end end add a feature adding a feature Test
99. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do seller = Seller.create(name: "Alpaca good seller", is_organic: true, is_fair_trade: true) not_included_seller = Seller.create(name: "Alpaca bad seller", is_organic: false, is_fair_trade: false) end end add a feature adding a feature Test
100. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do seller = Seller.create(name: "Alpaca good seller", is_organic: true, is_fair_trade: true) not_included_seller = Seller.create(name: "Alpaca bad seller", is_organic: false, is_fair_trade: false) end end adding a feature Test
101. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do seller = Seller.create(...) not_included_seller = Seller.create(...) Yarn.create(name: "alpaca", seller: seller, ...) Yarn.create(name: "alpaca", seller: not_included_seller, ...) end end add a feature adding a feature Test
102. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do // setup found_sellers = Yarn.get_sellers("alpaca") end end add a feature adding a feature Test
103. context "when you pass in alpaca as yarn_name" do it "returns sellers with only yarn that is fair trade and organic" do // setup found_sellers = Yarn.get_sellers("alpaca") expect(found_sellers.count).to eq 1 expect(found_sellers.first).to eq(seller) end end add a feature adding a feature Test
104. test fails add a feature adding a feature
105. NoMethodError: undefined method `count' for nil:NilClass adding a feature
106. when "alpaca" sellers = Seller.joins(:yarn).where(yarns: {name: "alpaca"}) add a feature adding a feature
107. when "alpaca" sellers = Seller.joins(:yarn).where(yarns: {name: "alpaca"}) found_sellers = [] sellers.each do |seller| ... end add a feature adding a feature
108. when "alpaca" sellers = Seller.joins(:yarn).where(yarns: {name: "alpaca"}) found_sellers = [] sellers.each do |seller| if(seller.is_organic? && seller.is_fair_trade?) ... end end add a feature adding a feature
109. when "alpaca" sellers = Seller.joins(:yarn).where(yarns: {name: "alpaca"}) found_sellers = [] sellers.each do |seller| if(seller.is_organic? && seller.is_fair_trade?) found_sellers << seller end end return found_sellers add a feature adding a feature
110. test passes add a feature adding a feature
111. but that was only the happy path add a feature adding a feature
112. what else could go wrong? add a feature adding a feature
113. when "alpaca" sellers = Seller.joins(:yarn).where(yarns: {name: "alpaca"}) found_sellers = [] sellers.each do |seller| if(seller.is_organic? && seller.is_fair_trade?) found_sellers << seller end end return found_sellers add a feature adding a feature
114. class Yarn < ApplicationRecord ... def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "wool", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: "wool"}) found_sellers << sellers end found_sellers.flatten! preferred_sellers = [] mid_sellers = [] found_sellers.each do |seller| if(seller.is_in_good_standing?) if(seller.is_organic? && seller.is_fair_trade?) preferred_sellers << seller elsif (seller.is_organic? && !seller.is_fair_trade?) mid_sellers << seller end end end if preferred_sellers.count > mid_sellers.count return preferred_sellers + mid_sellers elsif return found_sellers end adding a feature Don’t try to read this
115. adding a feature
116. refactor* *if you can adding a feature
117. why refactor? add a feature adding a feature
118. how do we refactor? add a feature adding a feature
119. class Yarn def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... when "cashmere" ... when "cotton" ... when "alpaca" ... when "silk" ... when "synthetic" ... Good opportunity to replace conditional with polymorphism adding a feature
120. class Yarn def self.get_sellers(name, gauge=nil, color=nil, weight=nil) found_sellers = [] case name when "wool" ... when "cashmere" ... when "cotton" ... when "alpaca" ... when "silk" ... when "synthetic" ... Good opportunity to replace conditional with subclasses adding a feature
121. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
122. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
123. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
124. fix a bug fixing a bug
125. A Common Taxonomy of Bugs and How to Catch Them Kylie Stradley fixing a bug
126. example fixing a bug
127. sellers with silk blends are not being returned fixing a bug THE BUG
128. write a test to duplicate the bug fixing a bug
129. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do ... end end fixing a bug Test
130. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do seller = Seller.create(name: "Silk seller", is_organic: true) silk_blend_seller = Seller.create(name: "Silk blend seller", is_organic: true,) end end fixing a bug Test
131. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do seller = ... silk_blend_seller = ... Yarn.create(name: "silk", seller: seller, is_organic:true, ..) Yarn.create(name: "silk-blend", seller: silk_blend_seller, is_organic:true,...) end end fixing a bug Test
132. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do seller = ... silk_blend_seller = ... Yarn.create(name: "silk", seller: seller, is_organic:true, ..) Yarn.create(name: "silk-blend", seller: silk_blend_seller, is_organic:true,...) end end fixing a bug Test
133. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do //setup found_sellers = Yarn.get_sellers("silk") end end fixing a bug Test
134. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do //setup found_sellers = Yarn.get_sellers("silk") expect(found_sellers.count).to eq 2 expect(found_sellers).to include(silk_blend_seller) end end fixing a bug Test
135. test fails add a feature fixing a bug
136. Yarn#get_seller when you pass in silk as yarn_name returns sellers with silk and silk blend yarns Failure/Error: expect(found_sellers.count).to eq 2 expected: 2 got: 1 (compared using ==) add a feature fixing a bug
137. add code to make your test pass fixing a bug
138. when "silk" ... sellers = Seller.joins(:yarn).where(yarns: { name: "silk" }) found_sellers << sellers end fixing a bug
139. when "silk" ... sellers = Seller.joins(:yarn).where(yarns: { name: ["silk", "silk-blend"] }) found_sellers << sellers end fixing a bug
140. test passes! add a feature fixing a bug
141. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
142. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
143. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
144. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
145. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
146. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
147. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
148. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers elsif(color && !gauge && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color }) found_sellers << sellers elsif(weight && !gauge && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: weight }) found_sellers << sellers elsif(gauge && color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color:color }) found_sellers << sellers elsif(gauge && weight && !color) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, weight:weight }) found_sellers << sellers elsif (color && weight && !gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", color: color, weight: weight }) found_sellers << sellers elsif (color && weight && gauge) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge, color: color, weight: weight }) found_sellers << sellers else sellers = Seller.joins(:yarn).where(yarns: {name: ["silk", "silk-blend"]}) found_sellers << sellers end fixing a bug
149. when "silk" if(gauge && !color && !weight) sellers = Seller.joins(:yarn).where(yarns: { name: "silk", gauge: gauge }) found_sellers << sellers ... fixing a bug
150. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do seller = ... silk_blend_seller = ... Yarn.create(name: "silk", seller: seller, is_organic:true, gauge: 6, ...) Yarn.create(name: "silk-blend", seller: silk_blend_seller, is_organic:true, gauge: 6, ...) end end fixing a bug Test
151. context "when you pass in silk as yarn_name" do it "returns sellers with silk and silk blend yarns" do //setup found_sellers = Yarn.get_sellers("silk", 6) expect(found_sellers.count).to eq 2 expect(found_sellers).to include(silk_blend_seller) end end fixing a bug Test
152. test fails add a feature fixing a bug
153. Yarn#get_seller when you pass in silk as yarn_name returns sellers with silk and silk blend yarns Failure/Error: expect(found_sellers.count).to eq 2 expected: 2 got: 1 (compared using ==) add a feature fixing a bug
154. when "silk" ... sellers = Seller.joins(:yarn).where(yarns: { name: ["silk", "silk-blend"], gauge: gauge }) found_sellers << sellers end fixing a bug
155. test passes! add a feature fixing a bug
156. refactor add a feature fixing a bug
157. run all of the tests fixing a bug
158. what if something broke? fixing a bug
159. FIRST: this is good. fixing a bug
160. SECOND: what failed? fixing a bug
161. THIRD: check the assumptions of your new code. fixing a bug
162. Table of Contents 1. What’s up with legacy code? 2. Pain points a. Adding a Feature b. Fixing a Bug 3. Lessons Learned 4. Other recommendations
163. lessons learned
164. lessons learned add tests when you touch code that is currently untested.
165. lessons learned add tests when you touch code that is currently untested. BONUS: THIS WILL HELP YOU LEARN QUICKLY THINGS ABOUT HOW YOUR APP WORKS
166. refactor to improve design incrementally. lessons learned
167. ask for help. lessons learned
168. add tests for new functionality. lessons learned
169. other recommendations Refactoring: Improving the Design of Existing Code Martin Fowler Working Effectively With Legacy Code Michael C. Feathers Object Oriented Design In Ruby Sandi Metz Apprenticeship Patterns Adewale Oshineye and Dave Hoover A Common Taxonomy of Bugs and How to Catch Them Kylie Stradley Get a Whiff of This Sandi Metz https://sourcemaking.com/refactoring/smells/
170. thank you! <may I put your name and picture here?>