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

An Optimistic Proposal for Making Horrible Code... Bearable

An Optimistic Proposal for Making Horrible Code... Bearable

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.

In this session, 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.

In the thousand mile journey, here are the first steps.

Joseph Mastey

April 27, 2017
Tweet

More Decks by Joseph Mastey

Other Decks in Programming

Transcript

  1. @jmmastey
    "O0QUJNJTUJD1SPQPTBMGPS
    .BLJOH)PSSJCMF$PEF
    #FBSBCMF
    Joe Mastey
    RailsConf 2017

    View full-size slide

  2. @jmmastey
    • 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
    NPQMBUGPSNT
    NPQSPCMFNT

    View full-size slide

  4. @jmmastey
    PVSPXOWJDUJNT

    View full-size slide

  5. @jmmastey
    CVUXIZ

    View full-size slide

  6. @jmmastey
    XFDBO`UUFMMXIBUDPEF
    JTFWFOJOVTFBOZNPSF

    View full-size slide

  7. @jmmastey
    DIBOHJOHDPEFCSFBLT
    DPNQMFUFMZVOSFMBUFEUFTUT

    View full-size slide

  8. @jmmastey
    HPPEMVDLNFSHJOHZPVS
    DPEFTUZMF13T

    View full-size slide

  9. @jmmastey
    69151 offenses detected

    View full-size slide

  10. @jmmastey
    XFDBO`UUSVTUUIF
    UFTUTVJUF

    View full-size slide

  11. @jmmastey
    TPXIBUEPUIFZEP
    JOTUFBE

    View full-size slide

  12. @jmmastey
    NBLFBUJOZDIBOHF

    View full-size slide

  13. @jmmastey
    DPNNJUJU
    TLJQUIFUFTUT

    View full-size slide

  14. @jmmastey
    SVOMJLFIFMM

    View full-size slide

  15. @jmmastey
    MFBSOFEIFMQMFTTOFTT

    View full-size slide

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

    View full-size slide

  17. @jmmastey
    OBNFUIFFWJM

    View full-size slide

  18. @jmmastey
    CBEOFTTJTVOFWFOMZ
    EJTUSJCVUFE

    View full-size slide

  19. @jmmastey
    wc -l $(find . -name *.rb) | sort

    View full-size slide

  20. @jmmastey
    … 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
    bundle exec rake stats

    View full-size slide

  23. @jmmastey
    +----------------------+-------+-------+---------+---------+-----+-------+
    | 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
    +----------------------+-------+-------+---------+---------+-----+-------+
    | 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
    +----------------------+-------+-------+---------+---------+-----+-------+
    | 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
    PSNBLFZPVSPXO

    View full-size slide

  27. @jmmastey
    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
    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. @jmmastey
    pYJOHUIFDPEF

    View full-size slide

  30. TUPQUIF
    CMFFEJOH

    View full-size slide

  31. @jmmastey
    # 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

  32. @jmmastey
    # ratchets_spec.rb
    ar_classes = ObjectSpace.each_object(Class)
    .select { |klass| klass < ActiveRecord::Base }
    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

  33. @jmmastey
    TUSBUFHJDJNQSPWFNFOUT

    View full-size slide

  34. pYJUJOQJFDFT

    View full-size slide

  35. CSFBLBHFJT
    HPJOHUP
    IBQQFO

    View full-size slide

  36. 1SJPSJUJ[F
    • make your tests better
    • eliminate dynamic code
    • behead the dragon
    • prioritize high churn code

    View full-size slide

  37. @jmmastey
    GPPMQSPPGUFTUpY

    View full-size slide

  38. @jmmastey
    EFMFUFUIFN

    View full-size slide

  39. @jmmastey
    NBLFXSJUJOHOFX
    UFTUTGBTU

    View full-size slide

  40. @jmmastey
    # require 'rails_helper'
    require 'spec_helper'

    View full-size slide

  41. @jmmastey
    FMJNJOBUFEZOBNJDDPEF

    View full-size slide

  42. @jmmastey
    strategy = user.payroll_strategy.constantize
    strategy.new(user, params)
    strategy.dispatch

    View full-size slide

  43. @jmmastey
    next_state = purchase_lifecycle.next_state
    purchase.send(:”mark_as_#{next_state}!")

    View full-size slide

  44. @jmmastey
    define_method
    constantize
    send
    method_missing
    eval (?!)

    View full-size slide

  45. @jmmastey
    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

  46. @jmmastey
    CFIFBEUIFESBHPO

    View full-size slide

  47. Badness
    Number of Files

    View full-size slide

  48. @jmmastey
    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
    # 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

  50. @jmmastey
    XBJU XUG

    View full-size slide

  51. @jmmastey
    69151 offenses detected

    View full-size slide

  52. @jmmastey
    1 file inspected, 36 offenses detected

    View full-size slide

  53. @jmmastey
    OBNFUIFFWJM
    TUPQUIFCMFFEJOH
    TUSBUFHJDBMMZJNQSPWF

    View full-size slide

  54. @jmmastey
    GPDVTPOUIFQSPDFTT

    View full-size slide

  55. @jmmastey
    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