Grow a bonsai, not a shrub

Grow a bonsai, not a shrub

Oftentimes we trade away code style for the sake of pushing new features. This will, more often than not, result in a tangled web of code that few understand and fewer can maintain. This talk explores Ruby’s ecosystem of static analysis tools, and how to wield them to trim your application’s code into the shape it should eventually take. Come see what application code that is neatly groomed looks like, and what tools you need to employ to get there.

8a66c2a7197be751b21ebd35319ec797?s=128

Kevin Deisz

October 10, 2018
Tweet

Transcript

  1. Grow a bonsai, not a shrub

  2. Kevin Deisz @kddeisz

  3. None
  4. Whose fault?

  5. the people that built automatic cars Whose fault?

  6. the people that built automatic cars the people that still

    rent out manual cars Whose fault?
  7. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  8. None of these

  9. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  10. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  11. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  12. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  13. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  14. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how Whose fault?
  15. the people that built automatic cars the people that still

    rent out manual cars your fault, for never learning how to drive a manual car even though your friend Greg told you multiple times he would teach you, your mother-in-law told you to pick a time and she would rent a manual car for the weekend and teach you, and your father is more than a little embarrassed that you don’t know how a societal failure to standardize Whose fault?
  16. Code is written by the first engineer that has to

    understand the problem.
  17. Code is read by every engineer that has to understand

    the problem.
  18. Bad outcomes

  19. Microservices Bad outcomes

  20. Microservices Halt and refactor Bad outcomes

  21. Microservices Halt and refactor Complete rewrite Bad outcomes

  22. Microservices Halt and refactor Complete rewrite Bad outcomes

  23. Solutions

  24. Static Analysis Solutions

  25. Static Analysis
 Reflection Tests
 Solutions

  26. Static Analysis
 Reflection Tests
 Strategic Investment
 Solutions

  27. Static Analysis
 Determine the standards Reflection Tests
 Strategic Investment
 Solutions

  28. Static Analysis
 Determine the standards Reflection Tests
 Enforce standards on

    new code Strategic Investment
 Solutions
  29. Static Analysis
 Determine the standards Reflection Tests
 Enforce standards on

    new code Strategic Investment
 Enforce standards on old code Solutions
  30. Static Analysis • Reflection Tests • Strategic Investment

  31. Static Analysis Static Analysis • Reflection Tests • Strategic Investment

    Determine the standards
  32. rubocop Static Analysis • Reflection Tests • Strategic Investment

  33. rubocop Static Analysis • Reflection Tests • Strategic Investment

  34. rubocop Static Analysis • Reflection Tests • Strategic Investment

  35. Bundler Gemspec Layout Lint Metrics rubocop Static Analysis • Reflection

    Tests • Strategic Investment Naming Performance Rails Security Style
  36. Metrics rubocop Static Analysis • Reflection Tests • Strategic Investment

  37. AbcSize BlockLength BlockNesting ClassLength CyclomaticComplexity Metrics Static Analysis • Reflection

    Tests • Strategic Investment LineLength MethodLength ModuleLength ParameterLists PerceivedComplexity
  38. reek Static Analysis • Reflection Tests • Strategic Investment

  39. Attribute Boolean Parameter Class Variable Control Parameter Data Clump Duplicate

    Method Call Feature Envy Instance Variable Assumption Irresponsible Module Long Parameter List reek Long Yield List Manual Dispatch Missing Safe Method Module Initialize Nested Iterators Nil Check Repeated Conditional Subclassed From Core Class Too Many Constants Too Many Instance Variables Too Many Methods Too Many Statements Uncommunicative Method Name Uncommunicative Module Name Uncommunicative Parameter Name Uncommunicative Variable Name Unused Parameters Unused Private Method Utility Function Static Analysis • Reflection Tests • Strategic Investment
  40. reek Long Yield List Manual Dispatch Missing Safe Method Module

    Initialize Nested Iterators Nil Check Repeated Conditional Subclassed From Core Class Too Many Constants Too Many Instance Variables Too Many Methods Too Many Statements Uncommunicative Method Name Uncommunicative Module Name Uncommunicative Parameter Name Uncommunicative Variable Name Unused Parameters Unused Private Method Utility Function Static Analysis • Reflection Tests • Strategic Investment Attribute Boolean Parameter Class Variable Control Parameter Data Clump • Duplicate Method Call Feature Envy Instance Variable Assumption • Irresponsible Module Long Parameter List
  41. Rules Static Analysis • Reflection Tests • Strategic Investment BlockLength

    ClassLength LineLength ModuleLength MethodLength Too Many Constants Too Many Methods Too Many Statements AbcSize BlockNesting Cyclomatic Complexity Long Parameter List Long Yield List Nested Iterators ParameterLists Perceived Complexity Feature Envy Too Many Instance Variables Utility Function
  42. Rules - variance Static Analysis • Reflection Tests • Strategic

    Investment BlockLength ClassLength LineLength ModuleLength MethodLength Too Many Constants Too Many Methods Too Many Statements
  43. Rules - complexity Static Analysis • Reflection Tests • Strategic

    Investment AbcSize BlockNesting Cyclomatic Complexity Long Parameter List Long Yield List Nested Iterators ParameterLists Perceived Complexity
  44. Rules - design Static Analysis • Reflection Tests • Strategic

    Investment Feature Envy Too Many Instance Variables Utility Function
  45. Variance Static Analysis • Reflection Tests • Strategic Investment

  46. class Event ... def self.in_locations(locations) joins(in_locations_join_sources(locations)) .group(:id) .having(in_locations_match_conditions) end class

    << self def in_locations_explicit_location_match(locations) ... end def in_locations_join_sources(locations) ... end def in_locations_match_conditions ... end end end Static Analysis • Reflection Tests • Strategic Investment
  47. class Event ... def self.in_locations(locations) Queries::EventsInLocations.new(self, locations).results end end Static

    Analysis • Reflection Tests • Strategic Investment
  48. class Event ... def self.in_locations(locations) Queries::EventsInLocations.new(self, locations).results end end Static

    Analysis • Reflection Tests • Strategic Investment
  49. class Event ... def self.in_locations(locations) Queries::EventsInLocations.new(self, locations).results end end Static

    Analysis • Reflection Tests • Strategic Investment
  50. class Event ... def self.in_locations(locations) Queries::EventsInLocations.new(self, locations).results end end Static

    Analysis • Reflection Tests • Strategic Investment
  51. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end Static Analysis • Reflection Tests • Strategic Investment
  52. module Queries class EventsInLocations attr_reader :relation, :locations def initialize(relation, locations)

    @relation = relation @locations = locations end def results relation.joins(join_sources).group(:id) .having(match_conditions) end private def events @events ||= Event.arel_table end ... end end Static Analysis • Reflection Tests • Strategic Investment
  53. Complexity Static Analysis • Reflection Tests • Strategic Investment

  54. class User < ApplicationRecord ... def can_admin?(model) case model when

    Event model.host == self when Photo model.uploader == self || can_admin?(model.event) when Post model.creator == self when Recognition model.creator == self when Rsvp model.user == self || can_admin?(model.event) ... end end end Static Analysis • Reflection Tests • Strategic Investment
  55. class User < ApplicationRecord ... def can_admin?(model) # rubocop:disable all

    case model when Event model.host == self when Photo model.uploader == self || can_admin?(model.event) when Post model.creator == self when Recognition model.creator == self when Rsvp model.user == self || can_admin?(model.event) ... end end end Static Analysis • Reflection Tests • Strategic Investment
  56. class User < ApplicationRecord ... def can_admin?(model) case model when

    Event model.host == self when Photo model.uploader == self || can_admin?(model.event) when Post model.creator == self when Recognition model.creator == self when Rsvp model.user == self || can_admin?(model.event) ... end end end Static Analysis • Reflection Tests • Strategic Investment
  57. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  58. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  59. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  60. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  61. module Users class AdminCheck attr_reader :user def initialize(user) @user =

    user end def can_admin?(model) case model when Event model.host == user when Photo model.uploader == user || can_admin?(model.event) when Post model.creator == user when Recognition model.creator == user ... end end end Static Analysis • Reflection Tests • Strategic Investment
  62. Design Static Analysis • Reflection Tests • Strategic Investment

  63. module Users class AdminCheck attr_reader :user def initialize(user) @user =

    user end def can_admin?(model) case model when Event model.host == user when Photo model.uploader == user || can_admin?(model.event) when Post model.creator == user ... end end end Static Analysis • Reflection Tests • Strategic Investment
  64. module Users class AdminCheck ... def can_admin?(model) case model when

    Event model.host == user when Photo model.uploader == user || can_admin?(model.event) when Post model.creator == user ... end end end Static Analysis • Reflection Tests • Strategic Investment
  65. module Checks refine Event do def admin?(user) host == user

    end end end module Users class AdminCheck ... using Checks def can_admin?(model) case model when Event model.admin?(user) when Photo model.uploader == user || can_admin?(model.event) when Post model.creator == user ... end end end Static Analysis • Reflection Tests • Strategic Investment
  66. module Checks refine Event do def admin?(user) host == user

    end end refine Photo do def admin?(user) uploader == user || event.admin?(user) end end end Static Analysis • Reflection Tests • Strategic Investment module Users class AdminCheck ... using Checks def can_admin?(model) case model when Event model.admin?(user) when Photo model.admin?(user) when Post model.creator == user ... end end end
  67. module Checks refine Event do def admin?(user) host == user

    end end refine Photo do def admin?(user) uploader == user || event.admin?(user) end end refine Post do def admin?(user) creator == user end end end Static Analysis • Reflection Tests • Strategic Investment module Users class AdminCheck ... using Checks def can_admin?(model) case model when Event model.admin?(user) when Photo model.admin?(user) when Post model.admin?(user) ... end end end
  68. module Checks refine Event do def admin?(user) host == user

    end end refine Photo do def admin?(user) uploader == user || event.admin?(user) end end refine Post do def admin?(user) creator == user end end ... end module Users class AdminCheck ... using Checks def can_admin?(model) model.admin?(user) end end end Static Analysis • Reflection Tests • Strategic Investment
  69. Extract common behavior Static Analysis • Reflection Tests • Strategic

    Investment
  70. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end Static Analysis • Reflection Tests • Strategic Investment
  71. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end Static Analysis • Reflection Tests • Strategic Investment
  72. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end Static Analysis • Reflection Tests • Strategic Investment
  73. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end Static Analysis • Reflection Tests • Strategic Investment
  74. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  75. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  76. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  77. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  78. class Event ... scope :in_locations, -> (locations) { Queries::EventsInLocations.new(self, locations).results

    } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  79. class Event ... extend Ext::Query scope :in_locations, -> (locations) {

    Queries::EventsInLocations.new(self, locations).results } end module Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  80. class Event ... extend Ext::Query query :in_locations, Queries::EventsInLocations end module

    Ext module Query def query(name, constant) scope name, ->(*args) { constant.new(self, *args).results } end end end Static Analysis • Reflection Tests • Strategic Investment
  81. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  82. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  83. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  84. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    Static Analysis • Reflection Tests • Strategic Investment
  85. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  86. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  87. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  88. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  89. class User < ApplicationRecord ... def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end

    module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  90. Static Analysis • Reflection Tests • Strategic Investment class User

    < ApplicationRecord ... extend Ext::Factory def can_admin?(model) Users::AdminCheck.new(self).can_admin?(model) end end module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end
  91. class User < ApplicationRecord ... extend Ext::Factory factory :admin_check, Users::AdminCheck,

    delegate: :can_admin? end module Ext module Factory def factory(method_name, clazz, delegate: []) define_method(method_name) { clazz.new(self) } return if Array(delegate).empty? public_send(:delegate, *delegate, to: method_name) end end end Static Analysis • Reflection Tests • Strategic Investment
  92. Reflection Tests Static Analysis • Reflection Tests • Strategic Investment

    Enforce standards on new code
  93. Reflection tests should always answer a question. Static Analysis •

    Reflection Tests • Strategic Investment
  94. What needs to be changed before we can upgrade to

    the next version of Rails? Static Analysis • Reflection Tests • Strategic Investment
  95. What needs to be changed before we can upgrade to

    the next version of Rails? Static Analysis • Reflection Tests • Strategic Investment # Be sure to test this when we upgrade Rails
  96. What needs to be changed before we can upgrade to

    the next version of Rails? Static Analysis • Reflection Tests • Strategic Investment # Be sure to test this when we upgrade Rails # Monkey-patching Rails here, so we’ll need to
 # test this when we upgrade
  97. What needs to be changed before we can upgrade to

    the next version of Rails? Static Analysis • Reflection Tests • Strategic Investment # Be sure to test this when we upgrade Rails # We can remove this when we upgrade Rails # Monkey-patching Rails here, so we’ll need to
 # test this when we upgrade
  98. require 'test_helper' class UpgradeTest < ActiveSupport::TestCase LAST_TESTED_VERSION = Gem::Version.new('5.2.1') def

    self.test(name) version = Gem::Version.new(Rails::VERSION::STRING) super(name) { flunk if version > LAST_TESTED_VERSION } end test 'Comment#as_json' test 'counter_cache and touch on comment' test 'counter_cache and touch on cheer' test 'ApplicationController::make_response!' test 'Buildable::TouchAll' end Static Analysis • Reflection Tests • Strategic Investment
  99. require 'test_helper' class UpgradeTest < ActiveSupport::TestCase LAST_TESTED_VERSION = Gem::Version.new('5.2.1') def

    self.test(name) version = Gem::Version.new(Rails::VERSION::STRING) super(name) { flunk if version > LAST_TESTED_VERSION } end test 'Comment#as_json' test 'counter_cache and touch on comment' test 'counter_cache and touch on cheer' test 'ApplicationController::make_response!' test 'Buildable::TouchAll' end Static Analysis • Reflection Tests • Strategic Investment
  100. require 'test_helper' class UpgradeTest < ActiveSupport::TestCase LAST_TESTED_VERSION = Gem::Version.new('5.2.1') def

    self.test(name) version = Gem::Version.new(Rails::VERSION::STRING) super(name) { flunk if version > LAST_TESTED_VERSION } end test 'Comment#as_json' test 'counter_cache and touch on comment' test 'counter_cache and touch on cheer' test 'ApplicationController::make_response!' test 'Buildable::TouchAll' end Static Analysis • Reflection Tests • Strategic Investment
  101. require 'test_helper' class UpgradeTest < ActiveSupport::TestCase LAST_TESTED_VERSION = Gem::Version.new('5.2.1') def

    self.test(name) version = Gem::Version.new(Rails::VERSION::STRING) super(name) { flunk if version > LAST_TESTED_VERSION } end test 'Comment#as_json' test 'counter_cache and touch on comment' test 'counter_cache and touch on cheer' test 'ApplicationController::make_response!' test 'Buildable::TouchAll' end Static Analysis • Reflection Tests • Strategic Investment
  102. require 'test_helper' class UpgradeTest < ActiveSupport::TestCase LAST_TESTED_VERSION = Gem::Version.new('5.2.1') def

    self.test(name) version = Gem::Version.new(Rails::VERSION::STRING) super(name) { flunk if version > LAST_TESTED_VERSION } end test 'Comment#as_json' test 'counter_cache and touch on comment' test 'counter_cache and touch on cheer' test 'ApplicationController::make_response!' test 'Buildable::TouchAll' end Static Analysis • Reflection Tests • Strategic Investment
  103. When do we split actions out into a different controller?

    Static Analysis • Reflection Tests • Strategic Investment
  104. When do we split actions out into a different controller?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Split this out to a different controller
  105. When do we split actions out into a different controller?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Split this out to a different controller # TODO: Refactor permissions for this controller
  106. When do we split actions out into a different controller?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Split this out to a different controller # TODO: Share this logic # TODO: Refactor permissions for this controller
  107. require 'test_helper' Rails.application.eager_load! class ControllerActionsTest < ActiveSupport::TestCase ALLOWED = %w[index

    show create update destroy].freeze ApplicationController.descendants.each do |controller| test "#{controller.name} only has standard actions" do violations = controller.action_methods - ALLOWED assert_empty violations end end end Static Analysis • Reflection Tests • Strategic Investment
  108. require 'test_helper' Rails.application.eager_load! class ControllerActionsTest < ActiveSupport::TestCase ALLOWED = %w[index

    show create update destroy].freeze ApplicationController.descendants.each do |controller| test "#{controller.name} only has standard actions" do violations = controller.action_methods - ALLOWED assert_empty violations end end end Static Analysis • Reflection Tests • Strategic Investment
  109. require 'test_helper' Rails.application.eager_load! class ControllerActionsTest < ActiveSupport::TestCase ALLOWED = %w[index

    show create update destroy].freeze ApplicationController.descendants.each do |controller| test "#{controller.name} only has standard actions" do violations = controller.action_methods - ALLOWED assert_empty violations end end end Static Analysis • Reflection Tests • Strategic Investment
  110. require 'test_helper' Rails.application.eager_load! class ControllerActionsTest < ActiveSupport::TestCase ALLOWED = %w[index

    show create update destroy].freeze ApplicationController.descendants.each do |controller| test "#{controller.name} only has standard actions" do violations = controller.action_methods - ALLOWED assert_empty violations end end end Static Analysis • Reflection Tests • Strategic Investment
  111. require 'test_helper' Rails.application.eager_load! class ControllerActionsTest < ActiveSupport::TestCase ALLOWED = %w[index

    show create update destroy].freeze ApplicationController.descendants.each do |controller| test "#{controller.name} only has standard actions" do violations = controller.action_methods - ALLOWED assert_empty violations end end end Static Analysis • Reflection Tests • Strategic Investment
  112. Where do we place logic that belongs to ActiveRecord models?

    Static Analysis • Reflection Tests • Strategic Investment
  113. Where do we place logic that belongs to ActiveRecord models?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Move this into its own object
  114. Where do we place logic that belongs to ActiveRecord models?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Move this into its own object # TODO: Refactor to use new factory system
  115. Where do we place logic that belongs to ActiveRecord models?

    Static Analysis • Reflection Tests • Strategic Investment # TODO: Move this into its own object # TODO: Reuse logic from other location # TODO: Refactor to use new factory system
  116. require 'ripper' class DefList class DefStatement ... end ... def

    read(filepath) parse(filepath, Ripper.sexp(File.read(filepath))) end private def parse(filepath, node) defs << DefStatement.new(filepath, node) if node[0] == :def node.each do |subnode| parse(filepath, subnode) if subnode.is_a?(Array) end end end Static Analysis • Reflection Tests • Strategic Investment
  117. require 'ripper' class DefList class DefStatement ... end ... def

    read(filepath) parse(filepath, Ripper.sexp(File.read(filepath))) end private def parse(filepath, node) defs << DefStatement.new(filepath, node) if node[0] == :def node.each do |subnode| parse(filepath, subnode) if subnode.is_a?(Array) end end end Static Analysis • Reflection Tests • Strategic Investment
  118. require 'ripper' class DefList class DefStatement ... end ... def

    read(filepath) parse(filepath, Ripper.sexp(File.read(filepath))) end private def parse(filepath, node) defs << DefStatement.new(filepath, node) if node[0] == :def node.each do |subnode| parse(filepath, subnode) if subnode.is_a?(Array) end end end Static Analysis • Reflection Tests • Strategic Investment
  119. require 'ripper' class DefList class DefStatement ... end ... def

    read(filepath) parse(filepath, Ripper.sexp(File.read(filepath))) end private def parse(filepath, node) defs << DefStatement.new(filepath, node) if node[0] == :def node.each do |subnode| parse(filepath, subnode) if subnode.is_a?(Array) end end end Static Analysis • Reflection Tests • Strategic Investment
  120. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  121. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  122. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  123. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  124. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  125. Other reflection tests Static Analysis • Reflection Tests • Strategic

    Investment
  126. No multi-line scopes without using a query object Other reflection

    tests Static Analysis • Reflection Tests • Strategic Investment
  127. No multi-line scopes without using a query object No associations

    without automatic inverses or inverse_of specified Other reflection tests Static Analysis • Reflection Tests • Strategic Investment
  128. No multi-line scopes without using a query object No associations

    without automatic inverses or inverse_of specified No if statements Other reflection tests Static Analysis • Reflection Tests • Strategic Investment
  129. This is not heresy. Static Analysis • Reflection Tests •

    Strategic Investment
  130. It is heretical to expect code quality to be maintained

    exclusively through code review. Static Analysis • Reflection Tests • Strategic Investment
  131. Strategic Investment Static Analysis • Reflection Tests • Strategic Investment

    Enforce standards on old code
  132. Passing a linter
 is not a measure of code quality.

    Static Analysis • Reflection Tests • Strategic Investment
  133. Passing tests
 is not a measure of code quality. Static

    Analysis • Reflection Tests • Strategic Investment
  134. Using design patterns
 is not a measure of code quality.

    Static Analysis • Reflection Tests • Strategic Investment
  135. Code is written by the first engineer that has to

    understand the problem. Static Analysis • Reflection Tests • Strategic Investment
  136. Code is read by every engineer that has to understand

    the problem. Static Analysis • Reflection Tests • Strategic Investment
  137. Readability is the measure of the quality of the code

    that is going to change. Static Analysis • Reflection Tests • Strategic Investment
  138. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  139. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase LEGACY = File.readlines('legacy.txt').map(&:chomp).freeze test 'no defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) - LEGACY assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  140. Static Analysis • Reflection Tests • Strategic Investment require 'test_helper'

    class DefTest < ActiveSupport::TestCase LEGACY = File.readlines('legacy.txt').map(&:chomp).freeze test 'no more defs in models' do def_list = DefList.from(Dir['app/models/*.rb']) violations = def_list.defs.map(&:to_s) - LEGACY assert_empty violations, <<~MSG Expected #{methods} to be empty. It looks like you defined an instance method on an ActiveRecord model. We prefer to leave AR model declarations to contain only database-level logic (like scopes and associations) and to instead place other logic in delegated objects. Check out `Ext::Factory` and `Ext::Query` for examples. MSG end end
  141. When code is readable… Static Analysis • Reflection Tests •

    Strategic Investment
  142. features are shipped more quickly When code is readable… Static

    Analysis • Reflection Tests • Strategic Investment
  143. features are shipped more quickly engineers can onboard more quickly

    When code is readable… Static Analysis • Reflection Tests • Strategic Investment
  144. features are shipped more quickly engineers can onboard more quickly

    bus factor is reduced When code is readable… Static Analysis • Reflection Tests • Strategic Investment
  145. features are shipped more quickly engineers can onboard more quickly

    bus factor is reduced patterns become self-evident When code is readable… Static Analysis • Reflection Tests • Strategic Investment
  146. features are shipped more quickly engineers can onboard more quickly

    bus factor is reduced patterns become self-evident writing tests is easier When code is readable… Static Analysis • Reflection Tests • Strategic Investment
  147. Elegance is not a dispensable luxury but a factor that

    decides between success and failure. - Edsger Dijkstra Static Analysis • Reflection Tests • Strategic Investment
  148. Object-oriented programming is an exceptionally bad idea which could only

    have originated in California. - Edsger Dijkstra Static Analysis • Reflection Tests • Strategic Investment
  149. Static Analysis
 Determine the standards Reflection Tests
 Enforce standards on

    new code Strategic Investment
 Enforce standards on old code Solutions Static Analysis • Reflection Tests • Strategic Investment
  150. Static Analysis
 Determine the standards for future code Reflection Tests


    Enforce standards on new code Strategic Investment
 Enforce standards on old code Solutions Static Analysis • Reflection Tests • Strategic Investment
  151. Static Analysis
 Determine the standards for future code Reflection Tests


    Enforce standards on new code as it changes Strategic Investment
 Enforce standards on old code Solutions Static Analysis • Reflection Tests • Strategic Investment
  152. Static Analysis
 Determine the standards for future code Reflection Tests


    Enforce standards on new code as it changes Strategic Investment
 Enforce standards on old code if it needs to change Solutions Static Analysis • Reflection Tests • Strategic Investment
  153. Grow a bonsai, not a shrub github.com/kddeisz/bonsai Kevin Deisz @kddeisz