Maintaining a 6yo Project Simone Carle, / / DNSimple

@underwaterruby

Simone Carle4 @weppos

No content

Cycle of Maintenance

day 1

year 1

year 2

year 3

Game ver!

the DNSimple app

How big is it? It's hard to find reasonable code metrics

Lines of code are useful only when bragging to project managers.

➜ dnsimple git:(master) cloc . 4366 text files. 4357 unique files. 1174 files ignored. https://github.com/AlDanial/cloc v 1.66 T=22.99 s (139.3 files/s, 8110.2 lines/s) -------------------------------------------------------------------------------- Language files blank comment code -------------------------------------------------------------------------------- Ruby 2484 26061 5974 93289 JavaScript 18 3151 2460 15161 ERB 479 2187 0 14462 SASS 46 618 153 4253 SQL 4 2903 2685 4253 CoffeeScript 86 938 190 3043 XML 17 1 0 1918 JSON 46 32 0 1088 YAML 11 122 260 509 CSS 3 11 0 309 HTML 7 48 0 263 Bourne Again Shell 1 13 0 63 -------------------------------------------------------------------------------- SUM: 3202 36085 11722 138611 --------------------------------------------------------------------------------

+----------------------+-------+-------+---------+---------+-----+-------+ | Name | Lines | LOC | Classes | Methods | M/C | LOC/M | +----------------------+-------+-------+---------+---------+-----+-------+ | Controllers | 6481 | 4833 | 123 | 738 | 6 | 4 | | Helpers | 1825 | 1216 | 10 | 202 | 20 | 4 | | Models | 9540 | 6138 | 147 | 966 | 6 | 4 | | Mailers | 390 | 274 | 11 | 41 | 3 | 4 | | Javascripts | 2419 | 1850 | 36 | 490 | 13 | 1 | | Libraries | 14452 | 9877 | 340 | 1465 | 4 | 4 | | Api specs | 3519 | 2634 | 0 | 3 | 0 | 876 | | Clock specs | 19 | 14 | 0 | 0 | 0 | 0 | | Command specs | 8084 | 6066 | 3 | 4 | 1 | 1514 | | Controller specs | 10261 | 7710 | 0 | 1 | 0 | 7708 | | Feature specs | 143 | 109 | 0 | 3 | 0 | 34 | | Finder specs | 3873 | 2918 | 0 | 11 | 0 | 263 | | Helper specs | 1943 | 1502 | 2 | 1 | 0 | 1500 | | Integration specs | 32 | 25 | 0 | 0 | 0 | 0 | | Lib specs | 13906 | 10769 | 2 | 35 | 17 | 305 | | Mailer specs | 720 | 560 | 0 | 0 | 0 | 0 | | Model specs | 14719 | 11441 | 0 | 18 | 0 | 633 | | Processor specs | 1354 | 1074 | 0 | 0 | 0 | 0 | | Request specs | 5101 | 3843 | 0 | 6 | 0 | 638 | | Routing specs | 6088 | 4890 | 0 | 0 | 0 | 0 | | Serializer specs | 576 | 460 | 0 | 0 | 0 | 0 | | Validator specs | 609 | 476 | 6 | 9 | 1 | 50 | | Worker specs | 1320 | 941 | 0 | 0 | 0 | 0 | +----------------------+-------+-------+---------+---------+-----+-------+ | Total | 107374 | 79620 | 680 | 3993 | 5 | 17 | +----------------------+-------+-------+---------+---------+-----+-------+ Code LOC: 24188 Test LOC: 55432 Code to Test Ratio: 1:2.3

How can we keep our code maintainable?

convention |kənˈvɛnʃ(ə)n| 1 a way in which something is usually done: to attract the best patrons the movie houses had to ape the conventions and the standards of theatres. • [ mass noun ] behaviour that is considered acceptable or polite to most members of a society: he was an upholder of convention and correct form | [ count noun ] : the law is felt to express social conventions. 2 an agreement between states covering particular matters, especially one less formal than a treaty. the convention, signed by the six states bordering on the Black Sea, aims to prevent further pollution.

ConvenKons are important to ensure that the enKre team speaks the same language

the DNSimple wiki

the GitHub Ruby style guide h>ps:/ /github.com/styleguide/ruby the Ruby Style Guide h>ps:/ /github.com/bbatsov/ruby-style-guide

Rubocop

pattern |ˈpat(ə)n| 2 a regular and intelligible form or sequence discernible in the way in which something happens or is done: a complicating factor is the change in working patterns. 4 an excellent example for others to follow: he set the pattern for subsequent study.

Text paOerns Homogeneous text and names are easier to search and refactor.

No content

Code fragments Recurring behaviors or code fragments can become code snippets or be extracted.

No content

No content

PaOerns are useful to promote code reusability improve long-term maintainability and speed up code refactoring

4 principles

Experiment SomeHmes the code is not ready for extracHon.

Stand out When you experiment, write code that the team will noHce.

Bonus point: __ is also easy to grep!

"Stupid Names" Catch the a>enHon of the other developers.

No content

Measure Is a change really introducing quality?

Params

class ContactParams
 include ParamsAllowed
 
 def self.allowed
 [:label, :first_name, :last_name, :address, :city, :state, :postal_code, :country, :email, :phone]
 end
 end
 
 
 # Params encapsulate the logic to handle some parameters from an untrusted source.
 #
 # A Params object generally doesn't include the Param module directly. Instead, it relies to one of the
 # more specific implementations, such as ParamsAllowed.
 module Params
 def initialize(params)
 @params = ActiveSupport::HashWithIndifferentAccess.new(params.to_h)
 end
 end
 
 
 module ParamsAllowed
 def self.included(base)
 base.include Params
 end
 
 def to_h
 @params.slice(*self.class.allowed).to_hash.symbolize_keys!
 end
 end

h>ps:/ /codeclimate.com

Fail Experiments may fail!

Beware of Mixins

Pulling out methods from a large class into mixins is like hiding the dust under the carpet.

No content

– Anonymous "Any application with an app/concerns directory is concerning."

We're not afraid to be mistaken and learn from failed experiments.

Abstract Be in control of your code.

Custom API Any external dependency we introduce, we wrap it behind a custom API.

require 'phony'
 
 module Dnsimple
 
 # Phone is a wrapper for the current phone validation library.
 module Phone
 
 WHITELIST = {
 "+3912312345678" => "+39 12312345678"
 }
 
 # Makes a plausibility check on the number.
 def self.plausible?(number)
 whitelisted?(number) || 
 Phony.plausible?(number)
 end
 
 def self.format(number)
 whitelisted?(number) || 
 Phony.formatted(Phony.normalize(number), parentheses: false)
 end

• Side features or extensions can be introduced without an extensive change to the codebase • IncompaHbiliHes can be addressed in a single place • When and if needed, the external dependency can be replaced with li>le effort • TesHng doesn't require intensive stubbing

module Dnsimple
 class AliasResolver
 
 class << self
 def adapter
 @adapter ||= NullAdapter.new
 end
 
 def resolve(name)
 adapter.resolve(name)
 end
 
 def enable_test!
 self.adapter = test_adapter.new
 end
 end
 # NullAdapter is a special adapter
 # that discards every resolve request.
 class NullAdapter
 def resolve(name)
 Result.new(name, [], [])
 end
 end
 
 class TestAdapter
 def resolve(*)
 result = @stubs.shift
 result ? Result.new(*result) : Result.new
 end
 end
 
 class GoAdapter
 BIN = File.join(Rails.root, "bin", "dsalias")
 
 def resolve(name)
 #
 end
 end


Dnsimple::AliasResolver.enable_test!
 
 describe "a methods that interacts with the resolver" do
 before do
 AliasResolver.adapter.test :stub, "fully.qualified.host"
 end
 
 it "does something" do
 # ...
 end
 end


AcKveRecord We have special convenHons and guidelines.

• Methods defined in ActiveRecord::Base are not allowed outside the Models • Models MUST expose custom API to perform operaHons • Callbacks are allowed only for data integrity • Query methods in ActiveRecord::Base are not allowed outside the Models or Finders • Scopes can't be invoked directly outside Models or Finders and they exist only to support Finders

require 'model_finder'
 
 class SuffixFinder
 include ModelFinder::Finder
 
 def self.find(identifier)
 find_by_tld(identifier)
 end
 
 def self.find!(identifier)
 find(identifier) or not_found!(identifier)
 end
 
 def self.find_by_tld(identifier)
 tld = identifier.to_s.downcase
 enabled.where(tld: tld).take
 end
 
 def self.suffix_listing
 scope.enabled
 end
 def self.newgtld_suffixes
 scope.new_tlds.enabled.order_alpha
 end
 
 def self.gtld_suffixes
 scope.global_tlds.enabled.order_alpha
 end
 
 def self.cctld_suffixes
 scope.cc_tlds.enabled.order_alpha
 end
 
 def self.privacy_supported_tlds
 scope.where(whois_privacy: true).pluck(:tld)
 end
 
 private
 
 def self.not_found!(identifier)
 raise(ActiveRecord::RecordNotFound, "TLD `#{identifier}` not found")
 end
 
 end


These guidelines are the result of years of experiments, discussions and refactoring.

No content

No content

h>p:/ /hanamirb.org

module Controllers::Domains
 class Create
 def call(params)
 @result = DomainCreateCommand.execute(
 command_context, 
 this_account, 
 DomainParams.new(params))
 @domain = @result.data
 
 if @result.successful?
 render DomainSerializer.new(@domain), 201
 else
 render ErrorSerializer.new(@result.error, @domain) end
 end
 end
 end class DomainHostingController
 def create
 @result = DomainCreateCommand.execute(
 command_context,
 this_account,
 domain_params)
 @domain = @result.data
 
 if @result.successful?
 redirect_to domain_path(@domain)
 else
 format.html { render action: "new" }
 end
 end
 end

module Api::V2
 module Controllers::RegistrarWhoisPrivacy
 
 class Enable
 include Hanami::Action
 before :require_subscription!, :require_good_standing!
 
 def call(params)
 # code
 end
 end
 
 class Disable
 include Hanami::Action
 before :require_subscription!
 
 def call(params)
 # code
 end
 end
 
 end
 end


Wrapping up

Code Monkey Don't be a

Experiment Don't take my word for it

Fail It's ok. Be ready to fail and learn from failures.

Small steps Don't throw away your enHre codebase

Code Smells Pay a>enHon to

Programming Languages Get influenced by other

Start today!

Credits • h>ps:/ /www.flickr.com/photos/goya74/8584790873 • h>ps:/ /www.flickr.com/photos/81723459@N04/22559333751/

Thanks! DNSimple dnsimple.com @dnsimple Simone Carle4 simonecarle4.com @weppos