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

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.

Kevin Newton

October 10, 2018
Tweet

More Decks by Kevin Newton

Other Decks in Technology

Transcript

  1. Grow a bonsai,
    not a shrub

    View full-size slide

  2. Kevin Deisz
    @kddeisz

    View full-size slide

  3. Whose fault?

    View full-size slide

  4. the people that built automatic cars
    Whose fault?

    View full-size slide

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

    View full-size slide

  6. 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?

    View full-size slide

  7. None of these

    View full-size slide

  8. 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?

    View full-size slide

  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?

    View full-size slide

  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?

    View full-size slide

  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?

    View full-size slide

  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?

    View full-size slide

  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?

    View full-size slide

  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
    a societal failure to standardize
    Whose fault?

    View full-size slide

  15. Code is written by the first
    engineer that has to
    understand the problem.

    View full-size slide

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

    View full-size slide

  17. Bad outcomes

    View full-size slide

  18. Microservices
    Bad outcomes

    View full-size slide

  19. Microservices
    Halt and refactor
    Bad outcomes

    View full-size slide

  20. Microservices
    Halt and refactor
    Complete rewrite
    Bad outcomes

    View full-size slide

  21. Microservices
    Halt and refactor
    Complete rewrite
    Bad outcomes

    View full-size slide

  22. Static Analysis
    Solutions

    View full-size slide

  23. Static Analysis

    Reflection Tests

    Solutions

    View full-size slide

  24. Static Analysis

    Reflection Tests

    Strategic Investment

    Solutions

    View full-size slide

  25. Static Analysis

    Determine the standards
    Reflection Tests

    Strategic Investment

    Solutions

    View full-size slide

  26. Static Analysis

    Determine the standards
    Reflection Tests

    Enforce standards on new code
    Strategic Investment

    Solutions

    View full-size slide

  27. Static Analysis

    Determine the standards
    Reflection Tests

    Enforce standards on new code
    Strategic Investment

    Enforce standards on old code
    Solutions

    View full-size slide

  28. Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  29. Static Analysis
    Static Analysis • Reflection Tests • Strategic Investment
    Determine the standards

    View full-size slide

  30. rubocop
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  31. rubocop
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  32. rubocop
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  33. Bundler
    Gemspec
    Layout
    Lint
    Metrics
    rubocop
    Static Analysis • Reflection Tests • Strategic Investment
    Naming
    Performance
    Rails
    Security
    Style

    View full-size slide

  34. Metrics
    rubocop
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  35. AbcSize
    BlockLength
    BlockNesting
    ClassLength
    CyclomaticComplexity
    Metrics
    Static Analysis • Reflection Tests • Strategic Investment
    LineLength
    MethodLength
    ModuleLength
    ParameterLists
    PerceivedComplexity

    View full-size slide

  36. reek
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  42. Rules - design
    Static Analysis • Reflection Tests • Strategic Investment
    Feature Envy
    Too Many
    Instance
    Variables
    Utility Function

    View full-size slide

  43. Variance
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  51. Complexity
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  60. Design
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  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
    ...
    end
    end
    end
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  67. Extract common
    behavior
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  90. Reflection Tests
    Static Analysis • Reflection Tests • Strategic Investment
    Enforce standards on new code

    View full-size slide

  91. Reflection tests should
    always answer a question.
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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
    # We can remove this when we upgrade Rails
    # Monkey-patching Rails here, so we’ll need to

    # test this when we upgrade

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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
    # TODO: Share this logic
    # TODO: Refactor permissions for this controller

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  123. Other reflection tests
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  124. No multi-line scopes without using a query object
    Other reflection tests
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  127. This is not heresy.
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  128. It is heretical to expect code
    quality to be maintained
    exclusively through code review.
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  129. Strategic Investment
    Static Analysis • Reflection Tests • Strategic Investment
    Enforce standards on old code

    View full-size slide

  130. Passing a linter

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

    View full-size slide

  131. Passing tests

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

    View full-size slide

  132. Using design patterns

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  135. Readability is the measure
    of the quality of the code
    that is going to change.
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  139. When code is readable…
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  140. features are shipped more quickly
    When code is readable…
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  145. Elegance is not a dispensable
    luxury but a factor that decides
    between success and failure.
    - Edsger Dijkstra
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

  146. Object-oriented programming is an
    exceptionally bad idea which could
    only have originated in California.
    - Edsger Dijkstra
    Static Analysis • Reflection Tests • Strategic Investment

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  151. Grow a bonsai,
    not a shrub
    github.com/kddeisz/bonsai
    Kevin Deisz
    @kddeisz

    View full-size slide