Thames Valley Meetup: Refactoring

Transcript

1. Refactoring Reafctoirng
2. @Jack_Franklin
4. changing the design not the behaviour
8. beware, opinions!
9. "Any fool can write code for a computer to understand. Good programmers write code that humans can understand" ! Martin Fowler
11. // create the carousel ! carousel( 400, 500, $('img'), 1000, 2000, true );
12. function carousel( height, width, images, speed, delay, autoPlay ) { // code }
13. // create the carousel ! carousel({ height: 400, width: 500, … });
14. var h = 400; var w = 400; var play = true; var calc = function()… ! for (var key in things)
15. Name things after their intention
16. "There are only two hard things in Comp Sci, cache invalidation and naming things" ! Phil Karlton
17. var placePin = function(x, y) ! var getLatLon = function(x, y) ! var user = { coordinates: [x, y] }
18. var placePin = function(coords) ! var getLatLon = function(coords) ! var user = { coordinates: { x: 1, y: 2 } }
19. implicit knowledge
20. if I were to hand the code over to you, what do I have to explain?
21. var drawGraph = function(width, height) { width = 160 + width; height = 172.5 + height;
 }
23. ! var drawGraph = function(width, height) { var graphWidthPad = 160; width = graphWidthPad + width; …
 }
24. implicit knowledge is what trips future you up in 6 months time
25. function someFunc() { doSomething() and.then.something.else(); maybe.even.more(); var x = 2; var y = 3; keep.on.going(x); and.going.and.going(y); return on.and.on();
 }
26. strive for reusable, composable functions
27. this makes sure they do one thing and one thing well
28. and also makes them much easier to test
29. easy to test code is usually pretty good
30. var goToBeginning = function(carousel) { if(carousel.isAtEnd()) { carousel.goToStart();
 }
 }
31. var goToBeginning = function(carousel) { if(carousel.isAtEnd()) { carousel.goToStart();
 }
 }
32. carousel goToBeginning
33. carousel.goToBeginning = function() { if(this.isAtEnd()) { this.goToStart();
 }
 }
34. Component Component Component Component Component Component Component Component Component Component
35. components should know little about each other
36. one thing well
37. doing this in real life
38. 120 seconds
39. you will never get this right
40. you never know less about the problem
41. premature abstraction is the root of all evil
42. prefer duplication at first
43. /users ?created_at[gt]=2014-04-01 &created_at[lte]=2014-05-01 ! if params[:created_at][:gt] users = users.where("created_at >…") if params[:created_at][:lte] …
44. if params[:created_at][:gt] users = users.where("created_at >…") ! if params[:created_at][:lte] … ! if params[:created_at][:gte] … ! if params[:created_at][:lt] …
46. filters = params[:created_at] ! map = { lte: '<=', gt: '>', … } ! filters.reduce(User.all) do |col, (key, val)| sym = map[key] col.where("created_at #{sym} ?", …) end
48. you can be too clever for your own good
49. filters = params[:created_at] users = User.all ! filters.reduce(users) do |coll, (key, val)| case key when :lte then coll.where(…) when :gt then coll.where(…) … end
51. prefer clarity of intent over succinct code
52. if something goes wrong, back out
53. git commit all the time
54. you should rebase before pushing anyway
55. it's a slow, methodical, mechanical process
56. test driving features
57. Can a user subscribe? • NO if they are the owner of the blog • NO if they are an admin of the blog • NO if they are already subscribed to the blog • NO if the blog is private • Else, totally.
58. given input X I expect output Y
59. pure function! input X > same output Y no side effects
60. you should strive for pure functions
61. they are easy to test and less prone to causing large errors
62. Can a user subscribe? • NO if they are the owner of the blog • NO if they are an admin of the blog • NO if they are already subscribed to the blog • NO if the blog is private • Else, totally.
63. it "returns true if the user can" do res = UserSubscribe.can_subscribe? (user_id, blog) expect(res).to be(true) end
64. class UserSubscribe def self.can_subscribe? (user_id, blog) true end end
66. it "returns true if the user can" … ! it "returns false if the blog is private" …
68. class UserSubscribe def self.can_subscribe? (user_id, blog) !blog.private? end end
70. it "returns true if the user can" … ! it "returns false if the blog is private" … ! it "returns false if the user owns the blog" …
72. class UserSubscribe def self.can_subscribe?(user_id, blog) if blog.private? false elsif user.owns?(blog) false else true end end end
74. it "returns true if the user can" … ! it "returns false if the blog is private" … ! it "returns false if the user owns the blog" … ! it "returns false if the user is admin" …
75. class UserSubscribe def self.can_subscribe?(user_id, blog) if blog.private? false elsif user.owns?(blog) false elsif user.is_admin?(blog) false else true end end end
76. it "returns true if the user can" … ! it "returns false if the blog is private" … ! it "returns false if the user owns the blog" … ! it "returns false if the user is admin" … ! it "returns false if the user is subscribed" …
77. class UserSubscribe def self.can_subscribe?(user_id, blog) if blog.private? false elsif user.owns?(blog) false elsif user.is_admin?(blog) false elsif user.is_subscribed?(blog) false else true end end end
79. the first implementation doesn't matter
80. the first implementation is about understanding the problem
82. class UserSubscribe def self.can_subscribe?(user_id, blog) if blog.private? || user.owns?(blog) || user.is_admin?(blog) || user.is_subscribed?(blog) false else true end end end
83. class UserSubscribe def self.can_subscribe?(user_id, blog) ! !(blog.private? || user.owns?(blog) || user.is_admin?(blog) || user.is_subscribed?(blog)) ! end end
84. class UserSubscribe def self.can_subscribe?(user_id, blog) ! return false if blog.private? return false if user.owns?(blog) return false if … return false if … true end end
86. prefer clarity of intent over succinct code
87. TDD doesn't work every time
88. (but that doesn't mean you shouldn't write tests!)
89. but sometimes it might make sense to write the tests afterwards...
90. #TODOs don't get done
92. 49,732,824
93. Code Review
94. Code Review
96. Making a difficult change
97. refactor to make the change easy
98. make the change
99. RubyRogues #178 ! http://devchat.tv/ruby- rogues/episode-guide
100. Refactoring Book ! http://refactoring.com/
101. Thoughtbot Blog ! http:// robots.thoughtbot.com/
102. Thank You! ! @Jack_Franklin ! http://javascriptplayground.com/ the-refactoring-tales/