Thames Valley Meetup: Refactoring

Transcript

1. 1.

   Refactoring Reafctoirng

2. 2.

   @Jack_Franklin

3. 3.

   None
4. 4.

   changing the design not the behaviour

5. 5.

   None
6. 6.

   None
7. 7.

   None
8. 8.

   beware, opinions!

9. 9.

   "Any fool can write code for a computer to understand.

   Good programmers write code that humans can understand" ! Martin Fowler
10. 10.

    None
11. 11.

    // create the carousel ! carousel( 400, 500, $('img'), 1000,

    2000, true );
12. 12.

    function carousel( height, width, images, speed, delay, autoPlay ) {

    // code }
13. 13.

    // create the carousel ! carousel({ height: 400, width: 500,

    … });
14. 14.

    var h = 400; var w = 400; var play

    = true; var calc = function()… ! for (var key in things)
15. 15.

    Name things after their intention

16. 16.

    "There are only two hard things in Comp Sci, cache

    invalidation and naming things" ! Phil Karlton
17. 17.

    var placePin = function(x, y) ! var getLatLon = function(x,

    y) ! var user = { coordinates: [x, y] }
18. 18.

    var placePin = function(coords) ! var getLatLon = function(coords) !

    var user = { coordinates: { x: 1, y: 2 } }
19. 19.

    implicit knowledge

20. 20.

    if I were to hand the code over to you,

    what do I have to explain?
21. 21.

    var drawGraph = function(width, height) { width = 160 +

    width; height = 172.5 + height;
 }
22. 22.

    None
23. 23.

    ! var drawGraph = function(width, height) { var graphWidthPad =

    160; width = graphWidthPad + width; …
 }
24. 24.

    implicit knowledge is what trips future you up in 6

    months time
25. 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. 26.

    strive for reusable, composable functions

27. 27.

    this makes sure they do one thing and one thing

    well
28. 28.

    and also makes them much easier to test

29. 29.

    easy to test code is usually pretty good

30. 30.

    var goToBeginning = function(carousel) { if(carousel.isAtEnd()) { carousel.goToStart();
 }
 }

31. 31.

    var goToBeginning = function(carousel) { if(carousel.isAtEnd()) { carousel.goToStart();
 }
 }

32. 32.

    carousel goToBeginning

33. 33.

    carousel.goToBeginning = function() { if(this.isAtEnd()) { this.goToStart();
 }
 }

34. 34.

    Component Component Component Component Component Component Component Component Component Component

35. 35.

    components should know little about each other

36. 36.

    one thing well

37. 37.

    doing this in real life

38. 38.

    120 seconds

39. 39.

    you will never get this right

40. 40.

    you never know less about the problem

41. 41.

    premature abstraction is the root of all evil

42. 42.

    prefer duplication at first

43. 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. 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] …
45. 45.

    None
46. 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
47. 47.

    None
48. 48.

    you can be too clever for your own good

49. 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
50. 50.

    None
51. 51.

    prefer clarity of intent over succinct code

52. 52.

    if something goes wrong, back out

53. 53.

    git commit all the time

54. 54.

    you should rebase before pushing anyway

55. 55.

    it's a slow, methodical, mechanical process

56. 56.

    test driving features

57. 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. 58.

    given input X I expect output Y

59. 59.

    pure function! input X > same output Y no side

    effects
60. 60.

    you should strive for pure functions

61. 61.

    they are easy to test and less prone to causing

    large errors
62. 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. 63.

    it "returns true if the user can" do res =

    UserSubscribe.can_subscribe? (user_id, blog) expect(res).to be(true) end
64. 64.

    class UserSubscribe def self.can_subscribe? (user_id, blog) true end end

65. 65.

    None
66. 66.

    it "returns true if the user can" … ! it

    "returns false if the blog is private" …
67. 67.

    None
68. 68.

    class UserSubscribe def self.can_subscribe? (user_id, blog) !blog.private? end end

69. 69.

    None
70. 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" …
71. 71.

    None
72. 72.

    class UserSubscribe def self.can_subscribe?(user_id, blog) if blog.private? false elsif user.owns?(blog)

    false else true end end end
73. 73.

    None
74. 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. 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. 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. 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
78. 78.

    None
79. 79.

    the first implementation doesn't matter

80. 80.

    the first implementation is about understanding the problem

81. 81.

    None
82. 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. 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. 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
85. 85.

    None
86. 86.

    prefer clarity of intent over succinct code

87. 87.

    TDD doesn't work every time

88. 88.

    (but that doesn't mean you shouldn't write tests!)

89. 89.

    but sometimes it might make sense to write the tests

    afterwards...
90. 90.

    #TODOs don't get done

91. 91.

    None
92. 92.

    49,732,824

93. 93.

    Code Review

94. 94.

    Code Review

95. 95.

    None
96. 96.

    Making a difficult change

97. 97.

    refactor to make the change easy

98. 98.

    make the change

99. 99.

    RubyRogues #178 ! http://devchat.tv/ruby- rogues/episode-guide

100. 100.

     Refactoring Book ! http://refactoring.com/

101. 101.

     Thoughtbot Blog ! http:// robots.thoughtbot.com/

102. 102.

     Thank You! ! @Jack_Franklin ! http://javascriptplayground.com/ the-refactoring-tales/