Thames Valley Meetup: Refactoring

Transcript

1. Refactoring Reafctoirng

2. @Jack_Franklin

3. None

4. changing the design not the behaviour

5. None
6. None
7. None

8. beware, opinions!

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

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

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;
 }
22. None

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

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

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

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

65. None

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

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

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

69. None

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

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

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

79. the first implementation doesn't matter

80. the first implementation is about understanding the problem

81. None

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

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

91. None

92. 49,732,824

93. Code Review

94. Code Review

95. None

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/