Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Thames Valley Meetup: Refactoring

Aea964cf59c0c81fff752896f070cbbb?s=47 Jack Franklin
November 05, 2014

Thames Valley Meetup: Refactoring

Aea964cf59c0c81fff752896f070cbbb?s=128

Jack Franklin

November 05, 2014
Tweet

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/