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

An Optimistic Proposal for Making Horrible Code Bearable, V2

An Optimistic Proposal for Making Horrible Code Bearable, V2

How do we cope with really bad code?

How can we give our teams the confidence to work with legacy code that we don’t fully understand?

How can we stop the bleeding when we don’t have time to stop shipping?

The attempted rewrite is over, the dust has settled, and the monolith isn’t going away. After all, it’s still the app that makes all the money. On the other hand, nobody wants to work on it, every new feature takes forever, and your entire team is afraid of making any change for fear of the whole thing collapsing in on itself.

Join us for an exclusive event featuring Joe Mastey. During this event, we’ll walk through some of the technical and social problems that arise from difficult codebases. We’ll learn to stop making things worse, to measure what we need to change, and start making progress.

Joseph Mastey

June 01, 2017
Tweet

More Decks by Joseph Mastey

Other Decks in Programming

Transcript

  1. @jmmastey | http://bit.ly/2rw1W8I
    "O0QUJNJTUJD1SPQPTBMGPS
    .BLJOH)PSSJCMF$PEF
    #FBSBCMF
    Joe Mastey
    Nerd Interface June 2017

    View full-size slide

  2. @jmmastey | http://bit.ly/2rw1W8I
    • Custom system packages (my-libssl)
    • 6000+ line User model
    • Four days to run test suite, with 10% flapping tests.
    • 1,000,000+ lines of ruby

    View full-size slide

  3. @jmmastey | http://bit.ly/2rw1W8I
    XFDBO`UBMM
    CFIJQ

    View full-size slide

  4. @jmmastey | http://bit.ly/2rw1W8I
    PVSPXOWJDUJNT

    View full-size slide

  5. @jmmastey | http://bit.ly/2rw1W8I
    CVUXIZ

    View full-size slide

  6. @jmmastey | http://bit.ly/2rw1W8I
    XFDBO`UUFMMXIBUDPEF
    JTFWFOJOVTFBOZNPSF

    View full-size slide

  7. @jmmastey | http://bit.ly/2rw1W8I
    DIBOHJOHDPEFCSFBLT
    DPNQMFUFMZVOSFMBUFEUFTUT

    View full-size slide

  8. @jmmastey | http://bit.ly/2rw1W8I
    HPPEMVDLNFSHJOHZPVS
    DPEFTUZMF13T

    View full-size slide

  9. @jmmastey | http://bit.ly/2rw1W8I
    69151 offenses detected

    View full-size slide

  10. @jmmastey | http://bit.ly/2rw1W8I
    XFDBO`UUSVTUUIF
    UFTUTVJUF

    View full-size slide

  11. @jmmastey | http://bit.ly/2rw1W8I
    TPXIBUEPUIFZEP
    JOTUFBE

    View full-size slide

  12. @jmmastey | http://bit.ly/2rw1W8I
    NBLFBUJOZDIBOHF

    View full-size slide

  13. @jmmastey | http://bit.ly/2rw1W8I
    DPNNJUJU
    TLJQUIFUFTUT

    View full-size slide

  14. @jmmastey | http://bit.ly/2rw1W8I
    SVOMJLFIFMM

    View full-size slide

  15. @jmmastey | http://bit.ly/2rw1W8I
    MFBSOFEIFMQMFTTOFTT

    View full-size slide

  16. :PV"SF)FSF
    5IF5SPVHIPG%FTQBJS

    View full-size slide

  17. @jmmastey | http://bit.ly/2rw1W8I
    TUFQ
    OBNFUIFFWJM

    View full-size slide

  18. @jmmastey | http://bit.ly/2rw1W8I
    CBEOFTTJTVOFWFOMZ
    EJTUSJCVUFE

    View full-size slide

  19. @jmmastey | http://bit.ly/2rw1W8I
    wc -l $(find . -name *.rb) | sort

    View full-size slide

  20. @jmmastey | http://bit.ly/2rw1W8I
    … 220 other files here
    339 ./lib/devise/models/confirmable.rb
    347 ./test/integration/recoverable_test.rb
    350 ./test/models/lockable_test.rb
    363 ./test/integration/registerable_test.rb
    499 ./lib/devise.rb
    513 ./lib/devise/rails/routes.rb
    519 ./test/models/confirmable_test.rb
    698 ./test/integration/authenticatable_test.rb
    16262 total

    View full-size slide

  21. Badness
    Number of Files

    View full-size slide

  22. @jmmastey | http://bit.ly/2rw1W8I
    bundle exec rake stats

    View full-size slide

  23. @jmmastey | http://bit.ly/2rw1W8I
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Name | Lines | LOC | Classes | Methods | M/C | LOC/M |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Controllers | 3347 | 2570 | 71 | 310 | 4 | 6 |
    | Helpers | 432 | 347 | 0 | 49 | 0 | 5 |
    | Models | 1957 | 1451 | 51 | 200 | 3 | 5 |
    | Mailers | 85 | 71 | 3 | 9 | 3 | 5 |
    | Javascripts | 9344 | 6447 | 0 | 724 | 0 | 6 |
    | Libraries | 2196 | 1532 | 27 | 143 | 5 | 8 |
    | Controller specs | 1675 | 1237 | 0 | 5 | 0 | 245 |
    | …
    | View specs | 283 | 236 | 0 | 0 | 0 | 0 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Total | 24956 | 18451 | 153 | 1449 | 9 | 10 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    Code LOC: 12418 Test LOC: 6033 Code to Test Ratio: 1:0.5

    View full-size slide

  24. @jmmastey | http://bit.ly/2rw1W8I
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Name | Lines | LOC | Classes | Methods | M/C | LOC/M |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Controllers | 3347 | 2570 | 71 | 310 | 4 | 6 |
    | Helpers | 432 | 347 | 0 | 49 | 0 | 5 |
    | Models | 1957 | 1451 | 51 | 200 | 3 | 5 |
    | Mailers | 85 | 71 | 3 | 9 | 3 | 5 |
    | Javascripts | 9344 | 6447 | 0 | 724 | 0 | 6 |
    | Libraries | 2196 | 1532 | 27 | 143 | 5 | 8 |
    | Controller specs | 1675 | 1237 | 0 | 5 | 0 | 245 |
    | …
    | View specs | 283 | 236 | 0 | 0 | 0 | 0 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Total | 24956 | 18451 | 153 | 1449 | 9 | 10 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    Code LOC: 12418 Test LOC: 6033 Code to Test Ratio: 1:0.5

    View full-size slide

  25. @jmmastey | http://bit.ly/2rw1W8I
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Name | Lines | LOC | Classes | Methods | M/C | LOC/M |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Controllers | 3347 | 2570 | 71 | 310 | 4 | 6 |
    | Helpers | 432 | 347 | 0 | 49 | 0 | 5 |
    | Models | 1957 | 1451 | 51 | 200 | 3 | 5 |
    | Mailers | 85 | 71 | 3 | 9 | 3 | 5 |
    | Javascripts | 9344 | 6447 | 0 | 724 | 0 | 6 |
    | Libraries | 2196 | 1532 | 27 | 143 | 5 | 8 |
    | Controller specs | 1675 | 1237 | 0 | 5 | 0 | 245 |
    | …
    | View specs | 283 | 236 | 0 | 0 | 0 | 0 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    | Total | 24956 | 18451 | 153 | 1449 | 9 | 10 |
    +----------------------+-------+-------+---------+---------+-----+-------+
    Code LOC: 12418 Test LOC: 6033 Code to Test Ratio: 1:0.5

    View full-size slide

  26. @jmmastey | http://bit.ly/2rw1W8I
    PSNBLFZPVSPXO

    View full-size slide

  27. @jmmastey | http://bit.ly/2rw1W8I
    def methods_per_activerecord_class
    children = ObjectSpace.each_object(Class)
    .select { |klass| klass < ActiveRecord::Base }
    sorted = children.sort_by { |klass| klass.methods(false).count }
    sorted.reverse!
    sorted.map { |klass| "#{klass.methods(false).length} #{klass}" }
    end

    View full-size slide

  28. @jmmastey | http://bit.ly/2rw1W8I
    75 Business,
    74 User,
    59 Product,
    53 Group,
    48 JournalEntry,
    37 Notification,
    30 Order,
    30 Subscription,
    22 Audited::Audit,
    12 CombinedPurchaseRecord,
    8 PgSearch::Document

    View full-size slide

  29. TUFQ
    TUPQUIF
    CMFFEJOH

    View full-size slide

  30. @jmmastey | http://bit.ly/2rw1W8I
    # arrowhead of dooooom
    Metrics/BlockNesting:
    Max: 4
    # short methods, man...
    Metrics/MethodLength:
    Max: 414
    Metrics/LineLength:
    Max: 324
    # fewer parameters
    Metrics/ParameterLists:
    Max: 15

    View full-size slide

  31. @jmmastey | http://bit.ly/2rw1W8I
    # ratchets/models_spec.rb
    ar_classes = ObjectSpace.each_object(Class)
    .select { |klass| klass < ApplicationRecord }
    METHOD_THRESHHOLD = 505
    ar_classes.each do |klass|
    describe klass do
    it "shouldn't have more than #{METHOD_THRESHHOLD} methods" do
    expect(klass.methods(false).count).to be < METHOD_THRESHHOLD
    end
    end
    end

    View full-size slide

  32. @jmmastey | http://bit.ly/2rw1W8I
    TUFQ
    JNQSPWF TUSBUFHJDBMMZ

    View full-size slide

  33. @jmmastey | http://bit.ly/2rw1W8I
    pYJUJOQJFDFT

    View full-size slide

  34. @jmmastey | http://bit.ly/2rw1W8I
    CSFBLBHFJT
    HPJOHUP
    IBQQFO

    View full-size slide

  35. @jmmastey | http://bit.ly/2rw1W8I
    1SJPSJUJ[F
    • make your tests better
    • use less magic
    • cut off the head
    • prioritize high churn code

    View full-size slide

  36. @jmmastey | http://bit.ly/2rw1W8I
    GPPMQSPPGUFTUpY

    View full-size slide

  37. @jmmastey | http://bit.ly/2rw1W8I
    EFMFUFUIFN

    View full-size slide

  38. @jmmastey | http://bit.ly/2rw1W8I
    NBLFOFXUFTUTGBTU

    View full-size slide

  39. @jmmastey | http://bit.ly/2rw1W8I
    # require 'rails_helper'
    require 'spec_helper'

    View full-size slide

  40. @jmmastey | http://bit.ly/2rw1W8I
    VTFMFTTNBHJD

    View full-size slide

  41. @jmmastey | http://bit.ly/2rw1W8I
    strategy = user.payroll_strategy.constantize
    strategy.new(user, params)
    strategy.dispatch

    View full-size slide

  42. @jmmastey | http://bit.ly/2rw1W8I
    next_state = purchase_lifecycle.next_state
    purchase.send(:”mark_as_#{next_state}!")

    View full-size slide

  43. @jmmastey | http://bit.ly/2rw1W8I
    define_method
    constantize
    send
    method_missing
    eval (?!)

    View full-size slide

  44. @jmmastey | http://bit.ly/2rw1W8I
    strategy = case user.payroll_strategy
    when 'biweekly' then BiweeklyPayrollStrategy
    when 'fortnightly' then FortnightlyPayrollStrategy
    when 'monthly' then MonthlyPayrollStrategy
    end
    strategy.new(user, params)
    strategy.dispatch

    View full-size slide

  45. @jmmastey | http://bit.ly/2rw1W8I
    DVUPGGUIFIFBE

    View full-size slide

  46. Badness
    Number of Files

    View full-size slide

  47. @jmmastey | http://bit.ly/2rw1W8I
    # POST /api/users/create_from_facebook
    def create_from_facebook
    service = RegistersUserFromFacebook.new(params[:authentication_token])
    service.call
    @user = service.user
    if service.errors.empty?
    render status: :created, location: logged_in_profile_url
    else
    render json: { message: service.message, errors: service.errors },
    status: :unprocessable_entity
    end
    end

    View full-size slide

  48. @jmmastey | http://bit.ly/2rw1W8I
    class RegistersUserFromFacebook
    attr_reader :token, :user, :errors, :message
    def initialize(token)
    @token = token
    @errors = []
    end
    def call
    # 200 lines of doom here
    end
    end

    View full-size slide

  49. @jmmastey | http://bit.ly/2rw1W8I
    XBJU XUG

    View full-size slide

  50. @jmmastey | http://bit.ly/2rw1W8I
    69151 offenses detected

    View full-size slide

  51. @jmmastey | http://bit.ly/2rw1W8I
    1 file inspected, 36 offenses detected

    View full-size slide

  52. @jmmastey | http://bit.ly/2rw1W8I
    OBNFUIFFWJM
    TUPQUIFCMFFEJOH
    TUSBUFHJDBMMZJNQSPWF

    View full-size slide

  53. @jmmastey | http://bit.ly/2rw1W8I
    GPDVTPOUIFQSPDFTT

    View full-size slide

  54. @jmmastey | http://bit.ly/2rw1W8I


    View full-size slide

  55. @jmmastey | http://bit.ly/2rw1W8I
    0UIFS3FTPVSDFT
    • Refactoring Ruby (Jay Fields)

    • Why Programs Fail: A Guide to Systematic Debugging (Andreas Zeller)

    • Service Objects in Ruby

    • Gems:

    • rubocop / hound ci

    • rspec-activerecord-formatter

    • bundler-stats
    UIBOLT

    View full-size slide