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.

Transcript

1. Grow a bonsai, not a shrub
2. Kevin Deisz @kddeisz
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