An Optimistic Proposal for Making Horrible Code... Bearable

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

@jmmastey • Custom system packages (my-libssl) • 6000+ line User
model • Four days to run test suite, with 10% ﬂapping tests. • 1,000,000+ lines of ruby

@jmmastey NPQMBUGPSNT NPQSPCMFNT

@jmmastey PVSPXOWJDUJNT

@jmmastey CVUXIZ

@jmmastey XFDBO`UUFMMXIBUDPEF JTFWFOJOVTFBOZNPSF

@jmmastey DIBOHJOHDPEFCSFBLT DPNQMFUFMZVOSFMBUFEUFTUT

@jmmastey HPPEMVDLNFSHJOHZPVS DPEFTUZMF13T

@jmmastey 69151 offenses detected

@jmmastey XFDBO`UUSVTUUIF UFTUTVJUF

@jmmastey TPXIBUEPUIFZEP JOTUFBE

@jmmastey NBLFBUJOZDIBOHF

@jmmastey DPNNJUJU TLJQUIFUFTUT

@jmmastey SVOMJLFIFMM

@jmmastey MFBSOFEIFMQMFTTOFTT

:PV"SF)FSF 5IF5SPVHIPG%FTQBJS

@jmmastey OBNFUIFFWJM

@jmmastey CBEOFTTJTVOFWFOMZ EJTUSJCVUFE

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

@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

Badness Number of Files

@jmmastey bundle exec rake stats

@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

@jmmastey PSNBLFZPVSPXO

@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

@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

@jmmastey pYJOHUIFDPEF

TUPQUIF CMFFEJOH

6 *`NCBDL

@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

@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

@jmmastey TUSBUFHJDJNQSPWFNFOUT

pYJUJOQJFDFT

CSFBLBHFJT HPJOHUP IBQQFO

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

@jmmastey GPPMQSPPGUFTUpY

@jmmastey EFMFUFUIFN

@jmmastey NBLFXSJUJOHOFX UFTUTGBTU

@jmmastey # require 'rails_helper' require 'spec_helper'

@jmmastey FMJNJOBUFEZOBNJDDPEF

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

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

@jmmastey define_method constantize send method_missing eval (?!)

@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

@jmmastey CFIFBEUIFESBHPO

Badness Number of Files

@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

@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

@jmmastey XBJU XUG

@jmmastey 69151 offenses detected

@jmmastey 1 file inspected, 36 offenses detected

@jmmastey OBNFUIFFWJM TUPQUIFCMFFEJOH TUSBUFHJDBMMZJNQSPWF

@jmmastey GPDVTPOUIFQSPDFTT

@jmmastey

@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

An Optimistic Proposal for Making Horrible Code... Bearable

An Optimistic Proposal for Making Horrible Code... Bearable

More Decks by Joseph Mastey

Other Decks in Programming

Featured

Transcript