stick to the rules – adam nowak / @lubieniebieski – what rules?!



stick to the rules – adam nowak / @lubieniebieski – what rules?!

0 0


talk-rules


On Github lubieniebieski / talk-rules

stick to the rules

adam nowak / @lubieniebieski

@netguru

what rules?!

spoiler alert: this presentation will be about my code-related rules, most of them relatead to stuff I do working day to day with Ruby on Rails

don't be affraid to interrupt and discuss / ask questions

lots of the presented ideas are from the ideal world

he he.

no images

:(

some code examples

yay!

why rules?

I'm lazy. but I need to get things done.

it's good to stick to something

it's easier to make decisions (or even not to have to make ones)

we have OUR way at netguru - rules are good!

Sandi Metz' rules

Classes should be no longer than one hundred lines of code.

Methods should be no longer than five lines of code.

Pass no more than four parameters into a method. Hash options are parameters.

Controllers should instantiate only one object.

check a follow up in this post

http://robots.thoughtbot.com/sandi-metz-rules-for-developers

a few general rules

don't refactor too early

you might get unwanted results

respect code style guides

if you're unsure how to do something... just stick to the rules proposed by the community

https://github.com/bbatsov/ruby-style-guide

try to keep your work/code/focus scope as small as possible

think about communication

method names returning bool values

use ? sign

// some_view.haml
%h1= book.permissions.for_user(current_user) == 'read'

vs

# book.rb
def readable?
  # some code returning bool value
  true
end

# book.readable?

you should be able to read code like a newspaper

ask explicitly

if some_value ...
if !some_value ... # unless some_value
if items

vs

if sume_value.present? ...
if some_value.nil?
if items.any?

all if statements should operate on boolean (true/false) values

present? nil? any? all? exists? blank? empty?

Bang bang bang!

class Thing # it's formatted badly because it was hard to fit it here
  def break!
    @broken = true; self
  end
  def broken?
    @broken || false
  end
end

thing = Thing.new
#> Thing:0x007fdb6cae0a20
thing.broken?
#> false
thing.break!
#> Thing:0x007fdb6cae0a20 @broken=true
thing.broken?
#> true

Core-lib examples with ! equivalents:

downcase, upcase, merge, reject, etc.

context

Different responsibilities

class Post
  has_many :comments
end

vs

class Admin::Post
  validates :title, presence: true
end

same object in different contexts has a different meaning

naming variables...

... is hard!

but important!

name variables with regards to context

class RaceResult
  def initialize(user1, user2)
    ...
  end
end

vs

class RaceResult
  def initialize(winner, runnerup)
    ...
  end
end
RaceResult.new(biker_john, biker_sam)
RaceResult.new(honda_car, bmw_car)

name objects so someone else is able to understand its meaning

collection.each do |itm|
  itm.del!
end

vs

cart_items.each do |item|
  item.buy!
end

try to avoid too general naming for object holders (containers)

e.g. container, array, hash - just name it after items it contains

Feature Flags

use ff_* for naming features

it's easy to find

dependencies

pass all the items required by partial

- # app/views/users/some/nested/context/_user_info.haml
= friend.name

vs

- # app/views/friends/show.haml
...
= render 'user_info', user: friend
...

data migrations

logallthethings!

class UpdateUsersAddresses
  def up
    say "Number of users to update: #{User.count}"
    say_with_time 'Updating users addresses...' do
      ...
      if user_update.failed?
        say "User ##{user_update.user_id} refused to be updated"
      end
      ...
    end
  end
end
== 20141009094214 UpdateUsersAddressess: migrating =============
-- Number of users to update: 413
-- Updating users addresses...
-- User #666 refused to be updated
 -> 1.337s

Audit state pre and post execution

and check the results

Make it re-runnable

and don't be stressed when migration fails

Don't depend on AR relations

Remember about rollback migration

good approach

bin/rake db:migrate db:rollback && bin/rake db:migrate

http://robots.thoughtbot.com/workflows-for-writing-migrations-with-rollbacks-in-mind

read more:

wrap the data

use view objects

... or use decorators

https://github.com/drapergem/draper

... or use both

View Object

class ShoppingCartView
  attr_accessor :items

  def initialize(items)
    self.items = items
  end

  def total
    "#{items.sum(:price)} Rubies"
  end

  def popular_items
    items.select { |i| i.popular? }
  end
end

Decorator

class CartItemDecorator < Draper::Decorator
  def short_title
    object.title.truncate_words(5)
  end

  def li_css_class
    'popular' if popular?
  end
end

Template (view)

%h1 Shopping Cart
%ul
  - cart.items.each do |item|
    %li{ class: item.li_css_class }= item.short_title

= "#{cart.popular_items.count} of your items are very popular"
end
  • don't use models directly - decorate them
  • if you have complex a logic - create an object wrapping all the data you need on a page
  • it's good to have one object per page (it may contain other decorated objects)
  • no need to use nested data (post.author.address.city) - structure your wrapping object as you need it in the template

expose the data

check http://decentexposure.info/

no more `@` in controllers!

Comparison (1)

# app/controllers/users_controller.rb

class UsersController
  def index
    @users = User.all
  end
end

and view

%h1 Users:
= @users.count

Comparison (2)

# app/controllers/users_controller.rb

class UsersController
  helper_method :users

  def index; end

  private

  def users
    User.all
  end
end

and view

%h1 Users:
= users.count

Comparison (3)

# app/controllers/users_controller.rb

class UsersController
  expose(:users)
  # expose(:users) { User.all }

  def index; end
end

and view

%h1 Users:
= users.count

Extract data gathering

use repositiories

sample repository

class ItemRepository
  attr_accessor :user

  def initialize(user)
    self.user = user
  end

  def all
    Item.where(user_id: user.id)
  end

  def active
    all.where(active: true)
  end
end
ItemRepository.new(current_user).active

Why?

  • easy to test
  • easy to use
  • you can avoid scopes in model (good/bad)
  • use with AR first, then easy to migrate to API-based solution

Recent ideas

  • return collections as arrays, not AR::Relation
    • focus on optimization encapsulated it repository
    • no more n+1 queries?
  • no need for relations anymore?

want more?

use Search objects

Sample search object

class IdeaSearch
  search_on Idea
  searches :name, :popular

  def search_name
    search.where(name: name)
  end

  def search_popular
    search.where('users_count > 10')
  end
end
search = IdeaSearch.new(popular: true)
search.results

Check out Searchlight gem

https://github.com/nathanl/searchlight

thin models vs thin controllers

why not thin... everything?

multiple layers = a lot more objects, but it will be much easier to maintain

controller is the place where you put your dependencies

typical flow in controller

just displaying things

  • get data (model)
  • display data (model)

vs

  • get data (repository)
  • display data (view object / decorator)

Model

# ar/post.rb

class Post < ActiveRecord::Base
  belongs_to :author
  has_many :comments
end

Repository

# repositories/post_repository.rb

class PostRepository
  def all
    base_relation.order(author: :id)
  end

  private

  def base_relation
    Post.includes(:comments, :author)
  end
end

View Object

# view_objects/post_view.rb

class PostView
  attr_accessor :post
  delegate :title, :author, to: :post

  def initialize(post)
    self.post = post
  end

  def author_name
    author.name
  end

  def author_avatar
    gravatar(author.email)
  end
end

Controller

# controllers/posts_controller.rb

class PostsController
  def index
    @posts = decorated_posts
  end

  private

  def decorated_posts
    post_repository.all.map { |p| PostView.new(post) }
  end

  def post_repository
    PostRepository.new
  end
end

View

# views/posts/index.haml
%h1 All posts

%ul
  - @posts.each do |post|
    %li
      = post.title
      by
      = post.author_name

use services

typical service

class Users::Create
  attr_accessor :email, :user_mailer

  def initialize(email:, user_mailer = nil)
    self.email = email
    self.user_mailer = user_mailer || UserMailer.new
  end

  def call
    user = create_db_user(email)
    send_intro_email(user)
    user
  end

  private

  def create_db_user(email); end

  def send_intro_email(user)
    user_mailer.intro(user).deliver
  end
end

response objects

class Reponse::Success
  def success?
    true
  end
end
class Reponse::Error
  def success?
    false
  end
end
# ... service class

def call
  ...
  Response::Success.new
end

controller gets smaller

class UsersController
  def create
    user_creation = Users::Create.new(params[:user]).call
    if user_creation.success?
      render text: 'OK'
    else
      render text: 'NOT OK', status: :unprocessable_entity
    end
  end
end

services: summary

  • pass all the dependencies
  • use only one public method `call`
  • try them as special forces to do one job particulary well
  • plurar namespace `Users::*` - avoid namespace clashes
  • use response objects if needed
  • read Pewniak's blogpost

why this is hard?

you have to write more

you have to test more

you don't know when to start

14 developers before you were trying their approach

you haven't experienced the pain yet

... or you haven't heard the stories by people who have

test wisely

don't test private methods

they're about to change, those are only implementation details

don't stub too much

maybe use some dependency injections instead?

focus on contexts

always have your greenpath covered with acceptance tests

remember: buisness is the most important thing (almost all the time)

why is it beneficial?

distributed responsibility

tests are simpler

smaller scopes allows people to jump in very quickly

THE END

t.hanks!