Brown bag 1

Twitter: @krzkot

Github: lis2

Email: kotlarek.krzysztof@gmail.com

Why?

Ruby is flexible

  • Performance
  • Performance
  • Maintainability

what is needed is a mathematical technique that will provide a quantitative basis for modularization and allow us to identify software modules that will be difficult to test or maintain.
1976, Thomas J. McCabe

Cyclomatic Complexity Metric

Algorithm that counts the number of unique execution paths through a body of source code

Code also assigns values to variables and sends messages

Jerry Fitzpatrick published in 1997 "Applying the ABC Metric to C, C++, and Java"

  • Assignments is a count of variable assignments.
  • Branches counts not branches of an if statement (as one could forgivably infer) but branches of control, meaning function calls or message sends.
  • Conditions counts conditional logic.

Flog

Rubycritic

There should never be more than one reason for a method to change.

  • If you can’t tell exactly what a method does at a glance, it’s too long.
  • Methods with more than one level of nesting are usually too long.
  • Methods with a flog score of 10 or higher may be too long.

    class UserService
      def self.create_user!(registration_request)
        ActiveRecord::Base.transaction do
          is_new_record = !User.exists?(email: registration_request[:email])
          user = user_from_registration_request! registration_request
          return user, is_new_record
        end
      end
    end
      

    def create
      user, is_new_record = UserService.create_user!(user_hash)

      unless(is_new_record)
        return(render_unprocessable_entity('User already exists.'))
      end
      ...
    end
      

    def self.exists?(registration_request)
      !User.exists?(email: registration_request[:email])
    end

    def self.create_user!(registration_request)
      ActiveRecord::Base.transaction do
        user_from_registration_request! registration_request
      end
    end
      

    def create
      unless UserService.exists?(user_hash)
        return(render_unprocessable_entity('User already exists.'))
      end
      user = UserService.create_user!(user_hash)
      ...
    end

A class should have only one reason to change.
Martin Fowler

  • You can’t easily describe what the class does in one sentence.
  • You can’t tell what the class does without scrolling.
  • The class needs to change for more than one reason.
  • The class has more private methods than public methods.
  • The class has more than 7 methods.
  • The class has a total flog score of 50.
  • The class is changed more frequently than other classes in the application

The wrong object causes more trouble than no object at all
Avdi Grimm

Duplication is far cheaper than the wrong abstraction
Sandi Metz 


    module UserService
      def self.convert_subscription_request_to_user!(subscription_request)
      end

      def self.verify_registration_request!(registration)
      end

      def self.update_user!(update_request, scentregroup_id)
      end

      def self.update_user_from_registration!(registration, user)
      end
    end

Try REALLY hard only to have one public method.


      def self.convert_subscription_request_to_user!(subscription_request)
        activerecord::base.transaction do
          subscription_request.update!(confirmed_at: time.zone.now)
          user = user.find_by(email: subscription_request.email.downcase.squish)
          operation = profile_operation_updated

          if user.nil?
            operation = profile_operation_created
            user = create_user_from_subscription_request!(subscription_request)
          end

          create_interests!(subscription_request, user)
          [user, operation]
        end
      end
      

    class SubscriptionRequestToUserService
      attr_accessor :subscription_request, :user

      def initialize(subscription_request)
        @subscription_request = subscription_request
      end

      def call
        activerecord::base.transaction do
          confirm_request!

          user = get_user
          operation = get_operation(user)

          user ||= create_user_from_subscription_request!

          SubscriptionRequestToInterest.new(user, subscription_request).call

          [user, operation]
        end
      end

      private
      def confirm_request!
        subscription_request.update!(confirmed_at: time.zone.now)
      end

      def get_user
        @user ||= user.find_by(email: subscription_request.email.downcase.squish)
      end

      def get_operation(user)
        user ? profile_operation_updated : profile_operation_created
      end
    end
      

    class SubscriptionRequestToUserService
      def self.call(subscription_request)
        activerecord::base.transaction do
          confirm_request!(subscription_request)

          user = get_user(subscription_request)
          operation = get_operation(user)
          user ||= create_user_from_subscription_request!(subscription_request)

          SubscriptionRequestToInterest.new(subscription_request).call

          [user, operation]
        end
      end

      private
      def self.confirm_request!(subscription_request)
        subscription_request.update!(confirmed_at: time.zone.now)
      end

      def self.get_user(subscription_request)
        user.find_by(email: subscription_request.email.downcase.squish)
      end

      def self.get_operation(user)
        user ? profile_operation_created : profile_operation_updated
      end
    end

Nil, "" and "John" a.k.a. the good, the bad and the ugly


    user.first_name ||= profile.first_name
    user.last_name ||= profile.last_name
      

    if user.first_name.blank? && profile.first_name.present?
      user.first_name = profile.first_name
    end
    if user.last_name.blank? && profile.last_name.present?
      user.last_name = profile.last_name
    end

nilify_blanks


    class User < ActiveRecord::Base
      # sadly, it is a callback
      # nilify_blanks
      # nilify_blanks :types => [:text]
      # nilify_blanks :only => [:author, :title]
    end

Arity

  • You can’t easily change the method’s arguments.
  • The method has three or more arguments.
  • The method is complex due to number of collaborating parameters.
  • The method requires large amounts of setup during isolated testing.

        def self.create_phys_id!(user,
                                 ident,
                                 phys_provider,
                                 ext_identifier,
                                 minor_id,
                                 extra_details = {})
      

        def self.create_phys_id!(user,
                                 ident:,
                                 phys_provider:,
                                 ext_identifier:,
                                 minor_id:,
                                 extra_details: {})
      

        CreatePhysicalIdentifierService.call(params)

        # or parameter object

REST in peace

Devise

  • confirmations_controller.rb
  • omniauth_callbacks_controller.rb
  • passwords_controller.rb
  • registrations_controller.rb
  • sessions_controller.rb
  • unlocks_controller.rb

    get '/subscriptions', to: 'subscriptions#index'
    get '/subscriptions/options', to: 'subscriptions#options'
    get '/subscriptions/is_subscribed', to: 'subscriptions#is_subscribed'
    put '/subscriptions', to: 'subscriptions#update'
    delete '/subscriptions', to: 'subscriptions#destroy'
    post '/subscriptions/resubscribe', to: 'subscriptions#resubscribe'

    post '/unsubscribe', to: 'unsubscribes#create'
      

    resource :subscriptions, only: %i(index update destroy)
    get '/subscriptions/options', to: 'subscription_options#show'
    get '/subscriptions/is_subscribed', to: 'check_subscription#index'
    post '/subscriptions/resubscribe', to: 'resubscribe#create'
    post '/unsubscribe', to: 'unsubscribes#create'

active_interaction

ActiveInteraction gives you a place to put your business logic. If ActiveModel deals with your nouns, then ActiveInteraction handles your verbs. 

    require 'active_interaction'

    class Square < ActiveInteraction::Base
      float :x

      def execute
        x**2
      end
    end
      

    outcome = Square.run(x: 'two point one')
    outcome.valid?
    # => nil
    outcome.errors.messages
    # => {:x=>["is not a valid float"]}

    outcome = Square.run(x: 2.1)
    outcome.valid?
    # => true
    outcome.result
    # => 4.41
      

    def index
      @accounts = ListAccounts.run!
    end
      

    class ListAccounts < ActiveInteraction::Base
      def execute
        Account.not_deleted.order(last_name: :asc, first_name: :asc)
      end
    end
      

    interactions/
      pubsub_in/
        advam
        skyfii
      pubsub_out/
        parking
      registrations/
        create_user
        expire
        verify
      users/
        find_by_physical_identifer
        find_by_scentregroup_id
        find_by_email
        merge

80 Characters per line?

Timestamps

SubscriptionRequest

Presentations

http://www.railspoint.com/brown_bag1

  • flog, rubycritic, codeclimat?
  • single responsibility methods?
  • single responsibility services/classes?
  • Nilify Blanks
  • arity - 1-2 arguments + named attributes?
  • REST - 1 additional method, otherwise new controller?
  • ActiveInteraction
  • 160 charackters per line?
  • Timestamps
  • BrownBag presentations

Thank you