CSSスタイルガイドを作って良かった話

f:id:ringo_girl:20160516105300p:plain

こんにちは、デザインチームのミヤギ(@_ringogirl)です。

エンジニア目線の求人・転職サイト Forkwell Jobsでは、最近デザインのリニューアルを行いました(最近と言っても3ヶ月前の話ですが…)。 リニューアルに合わせてCSSのリビングスタイルガイドを作ることにしました。

実際に作っているスタイルガイドはここで公開しています

スタイルガイドとは

CSSのドキュメントのようなもので、サイトで使う色やタイポグラフィ、UIパターンなどを記述したものです。 見た目とコードをドキュメントとして読めるので、チームで開発するときの共有に役立ちます。

なぜスタイルガイドを作ったのか

僕が入社したのは約1年前なのですが、その頃からCSSがとっ散らかって肥大化してしまっていたので、CSSを触るのがつらい状況になっていました。 もちろんドキュメントも無かったので、どこにCSSコンポーネントが定義されているのか探さないといけないし、コードを書くまで見た目がわからない、色を定義された変数が多すぎて管理ができない、など様々な問題がありました。 このような状況を解決したかったのと、継続的にCSSをメンテナンスできる環境を作りたくてスタイルガイドを作ることにしました。また、エンジニアがデザインに関することで悩むことが減って、開発速度が上がればいいなという思いもあります。

どうやって作ったのか

f:id:ringo_girl:20160516105412p:plain

HologramというGemを使用して作っています。KSSなど有名なものもありますが、やることが少なくて複雑ではない印象だったのでHologramを選択しています。 技術的なことについては長くなってしまうような気がするので、別の記事で書いていきます。

スタイルガイドの管理方法

プロダクト上で作っていくのではなく、別リポジトリで開発をしています。 npmで管理して、そこからプロダクトに読み込むという方法をとりました。

スタイルガイドを作って良かったこと

コンポーネントのスコープを狭めることができた

プロダクトとスタイルガイドを分離することで、プロダクトのレイアウトなどに左右されないようなコンポーネントを書きやすくなりました。 機能ごとにコンポーネントを定義できるので、スコープが狭くなりメンテナンスしやすくなったと思います。

CSSの管理がしやすくなった

上でも書きましたが、スタイルガイドをnpmで読み込んでいるので管理しやすくなりました。 スタイルガイドの変更がプロダクト側にコミットされないので、コミットログの見通しもよくなった気がします。

エンジニアがある程度は楽できるようになった

まだまだ問題はありますが、以前よりはデザインに関することで悩むことが減ったんじゃないかなと思います。 チーム共通のドキュメントなので、コミュニケーションが取りやすくなった気がします。

新しい技術を学ぶことができた

ES2015やReactなど、スタイルガイドで新しい技術を学ぶことができました。Hologramのプラグインを書いて拡張することも楽しかったです。 肥大化したCSSに継ぎ足していくのはつらかったので、こういったところで楽しみができるのも良かったです。

今後の展望

スタイルガイドの運用は始まりましたが、不便なところや不十分なところがまだまだ多くあり、以下のようなことを考えていたりします。

  • Reactコンポーネントの提供
  • JSのテスト
  • CSSのテスト
  • CSSの再設計
  • 色の選定
  • OSS化

スタイルガイドは、今までの僕らの開発にはなかった仕組みでまだ手探りな部分もありますが、今後も継続して開発していけるように頑張りたいと思います!

Protected branches を使ったデプロイ自動化の始め方

初めまして、エンジニアチームの @app2641 です。
去年の夏に grooves へ入社しました。
今回は旬の新人がブログを書かせて頂きます。

さて、突然ですが皆さんはアプリケーションのデプロイをどのような方法で行っていますか?
Forkwell では master ブランチにプルリクがマージされたら capistrano を使って丹精込めて手作業でデプロイを行うということをやっていました。
ステージング環境で動作確認する際にも似たような方法を取っていて、正直なところこのデプロイ方法はだるいなあと感じていました。
僕のように日々ぽやーと作業している人間にとってはデプロイ先を間違えそうになったり、マージだけしておいてデプロイは明日になってからやろうとか考えて翌日すっかり忘れていたりなど散々なことになります。
そんな事態を避けるためにはどうすればいいか。
そうです、自動化すればいいんです!
機械で出来ることは機械に押し付けよう!

今回は Forkwell でデプロイを自動化した話をしたいと思います。

Wercker からデプロイ出来るようにする

Forkwell では CI に Wercker を使用しています。
Wercker はプライベートリポジトリであっても無料で利用出来て大変便利です。
便利なので今すぐに使いましょう。さあさあ。

wercker.com

wercker.yml の設定

まずは設定ファイルである wercker.yml にデプロイの方法を記述していきます。

deploy:
  steps:
    - script:
      name: mkdir -p $HOME/.ssh
      code: mkdir -p $HOME/.ssh
    - add-ssh-key:
        keyname: FORKWELL_SSH_KEY
    - create-file:
      name: deploy.sh
      filename: $HOME/deploy.sh
      overwrite: true
      content: |-
        set -e
        cd forkwell
        git pull origin master
        bundle install
        bundle exec cap production deploy
    - script:
      name: deploy to production
      code: ssh -o StrictHostKeyChecking=no $FORKWELL_USER@$FORKWELL_IP 'bash -s' < $HOME/deploy.sh

なんとなくやっていることは分かると思います。
Forkwell では踏み台サーバを経由してデプロイを行っているので、一度踏み台サーバへ接続し、そこから capistrano で本番環境へデプロイしています。
サーバを経由せずにデプロイしたいのであれば、

- cap:
  stage: production

と記述するだけで production へデプロイ出来ます。
create-filecap といった項目は steps と呼ばれるファンクションで他にも様々なものがあります。
詳しくは Wercker の ドキュメント を読むと理解が深まるでしょう。

Deploy target と変数の設定

ところで先ほどの wercker.yml の中で FORKWELL_SSH_KEY だとか FORKWELL_USER といった変数があったことに気付いたでしょうか。
Wercker では直接設定ファイルに書きたくない内容を変数として登録しておくことが出来ます。
SSH 鍵だとか AWS のキーなどは Wercker 側に登録しておくのがベターです。

また、テストが通るたび自由にデプロイされてはかなわないのでデプロイするブランチを限定する必要もあります。
そういった細かな設定は Wercker 側で行います。

SSH 鍵の生成

我々のようにどこぞのサーバに SSH してデプロイしたいのであればまずは SSH 鍵を生成しなくてはなりません。
まずは設定画面で SSH keys ページを開きます。
Generate new key pair ボタンから SSH 鍵を生成します。
f:id:app2641:20160301162022p:plain

こんな感じで鍵が生成されます。
生成した鍵は忘れずにデプロイ先のサーバへ登録しておきましょう。
f:id:app2641:20160301162150p:plain

Deploy target の設定

次に Targets ページを開いてデプロイの設定を行います。
Add deploy target というところのドロップダウンメニューから custom deploy を選択します。
f:id:app2641:20160301162221p:plain

適当な名前とデプロイを動かしたいブランチ名を入力して保存すればよいです。
f:id:app2641:20160301162243p:plain

続いて変数の設定です。
変数はテキスト系のものと SSH 鍵と二種類あります。

テキストの場合。
f:id:app2641:20160301162257p:plain

SSH 鍵の場合。
この例でいうと、 SOMETHING_PUBLIC という変数で公開鍵、SOMETHING_PRIVATE で秘密鍵を扱えるようになります。
f:id:app2641:20160301162309p:plain

これで master ブランチのテストが通ったあと自動でデプロイされるようになりました。
なんとも簡単です。

master ブランチへのマージ制約

自動デプロイの運用になると当たり前ですがテストが通ったあとすぐにデプロイが始まってしまいます。
つまりは non fast-forward マージの master ブランチを動作確認せずにデプロイすることがまま有りえるということです。
テストが通っているとはいえマージ後のブランチで動作確認しておかないと夜もおちおち眠れません。
git-flow なんかだと feature ブランチと master ブランチのあいだに develop ブランチを作って運用するのが筋ですが、プルリクを作る回数が増えて手間なので出来ればやりたくありません。
そこで我々のチームは GitHub が提供している protected branches の機能を使うことにしました。
protected branches を使えばテストが通っていないブランチや non fast-forward のブランチはマージ出来なくなります。
non fast-forward を禁止して fast-forward 出来る feature ブランチを動作確認しておけば安心してマージ出来るね、ということです。

設定はチェックボックスをチェックするだけの簡単なクリック業です。
f:id:app2641:20160301162323p:plain

これでテストが通っていても non fast-forward のブランチはマージ出来なくなりました。
しかも GitHub は便利なものでウェブ上で master ブランチを feature ブランチにマージする機能を提供しています。
プルリク画面で Update branch ボタンを押すと勝手にマージ出来る状態にしてくれます。すごいよすごいよー。
f:id:app2641:20160301162335p:plain

余談ではありますが、我々のチームでは GitHub の Deployments API を使ってステージングで動作確認をしたかどうかがプルリク画面で分かるようになっています。
あ、こいつ動作確認してねえなっていうのが分かるので素敵です。

f:id:app2641:20160301162343p:plain

まとめ

そういうわけでようやっと自動でデプロイする仕組みが出来上がりました。
Forkwell では Wercker を使っていますが CircleCI や TravisCI でも同様のことが簡単に出来るはずです。
自動化出来ることはどんどん機械に任せて、人間はコードを書くことに集中したいですね。
がんばっていきましょうー。
それではまた。

リファクタリングコンテスト in Ruby 審査結果発表

f:id:grooves:20160202205843p:plain:w400

大変長らくお待たせしました。Forkwell Jobs にて、2015年11月24日〜12月31日の期間で開催していた【リファクタリングコンテスト in Ruby 】の審査結果がようやく出揃いました。

今回、なんと最もよいコードに贈られる Ruby賞 を1人のユーザーが独占する結果となりました。

気になる審査結果の前に、あらためて審査員をご紹介します。

松田 明 ( @a_matsuda )

f:id:grooves:20160202195645j:plain:w100

Ruby / Rails / Haml / CarrierWave等のコミッター。kaminari / action_args / active_decorator / motorhead 等の作者。
好きな寿司:アナゴ

和田 卓人 ( @t_wada )

f:id:grooves:20160202195656p:plain:w100

タワーズ・クエスト株式会社取締役社長、プログラマ。日本におけるテスト駆動開発(TDD)のスペシャリスト。
好きな寿司:赤貝

藤村 大介 ( @ffu_ )

f:id:grooves:20160202195707j:plain:w100

grooves、Aiming、Quipperなどでリーダー的な活動をしていたプログラマー。RubyとHaskellが得意。フロントエンド開発の実績も豊富。
好きな寿司:コハダ

それでは審査結果の発表です。

Ruby賞

f:id:grooves:20160202195726p:plain:w400

今回 Ruby賞に選ばれたのは、imaz さんの2つの投稿です。
それぞれを見ていきましょう!

和田氏、藤村氏に最も良かったと選ばれたコード

imaz さんの"【Refactor Me】複数の配列の要素を並び替えるコード" をリファクタリングしました

Before

def sort_lists(a, b)
  a.sort!
  b.sort!
  b.size.times do |i|
    a.size.times do |j|
      x, y = a[j], b[i]
      if y < x
        a[j] = y
        b[i] = x
      else
        next
      end
    end
  end
  b.sort!

  [a, b]
end

require 'minitest/autorun'

class TestSortLists < Minitest::Test
  def test_sort_lists
    data = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].shuffle
    assert_equal([['a', 'b', 'c', 'd'], ['e', 'f', 'g', 'h']], sort_lists(data[0..3], data[4..7]))
  end
end

After

def sort_lists(a, b)
  c = [a, b].flatten.sort!

  [c.shift(a.length), c.shift(b.length)]
end

require 'minitest/autorun'

class TestSortLists < Minitest::Test
  def test_sort_lists
    data = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].shuffle
    assert_equal([['a', 'b', 'c', 'd'], ['e', 'f', 'g', 'h']], sort_lists(data[0..3], data[4..7]))
  end 

  def test_sort_lists_for_different_size_lists
    data = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'].shuffle
    assert_equal([['a', 'b', 'c'], ['d', 'e', 'f', 'g', 'h']], sort_lists(data[0..2], data[3..7]))
  end 
end

和田氏 コメント

「複数の配列の要素を並び替えるコード」に挑戦された方は何人かいらっしゃいましたが、
この回答は「簡潔に書ける」という Ruby の特色が遺憾なく発揮されており、
リファクタリング前のコードと比べて著しく可読性が改善されている点が印象に残りました。

ひとつ気になるのは、リファクタリング前の実装には存在していた副作用がなくなっている点です。
リファクタリング前のコードでソートされるのは戻り値だけでなく、引数に渡した配列も破壊的にソートされます。

リファクタリングで大事なのは「外部から見た振る舞いを変えずに、内部を改善する」ことなのですが、
この副作用は、はたして外部から見た振る舞いでしょうか?

今回は、引数に及ぼす副作用は、意図していないものであろうと判断しました。
理由は、リファクタリング前のテストに引数の副作用に関するテストが無いためです。
テストに書いていないということは、副作用は意図したものでない、という判断です。
テストが書かれている場合は、意図の有無を調べる材料として既存のテストを使い、
機能互換性に関する判断を行うことができます。
(このあたりの設計判断も回答に書かれているとなお良かったと思います)

などなど、様々な意味で、この投稿は印象に残りました。

藤村氏 コメント

Array#shift で副作用を起こしながら結果の配列を作るという、不変性とは何だったのか的なコードでテンションが上がりました。
トリッキーだと感じる人もいるかもしれませんが、僕は簡潔で読みやすいコードだと思います。ということであまり指摘するところが無いんですが、強いて言えばcという変数名はa、bの次にあるものを連想させてミスリーディングかもしれないので、sortedとかにするといいかなあと。

松田氏に最も良かったと選ばれたコード

imaz さんの"【Refactor Me】Controllerをスッキリさせたい" をリファクタリング

Before

class SessionsController < ApplicationController
  def signin
    @user = User.find_by(email: params[:email])

    if BCrypt::Password.new(@user.password_digest) == params[:password]
      self.current_user = @user
      redirect_to root_path
    else
      flash[:alert] = "Login failed."
      render :new
    end
  end
end

After

# app/controllers/sessions_controller.rb
class SessionsController < ApplicationController
  def signin(email, password)
    @user = User.find_by(email: email)

    if @user.authenticate(password)
      self.current_user = @user
      redirect_to root_path
    else
      flash.now[:alert] = "Login failed."
      render :new
    end
  end
end

# app/models/user.rb
class User < ActiveRecord::Base
  has_secure_password validations: false
end

松田氏 コメント

バグを出さないおそらく唯一の方法は、プログラムを自分で書かないことです。
特にチームでのアプリケーション開発では、フレームワークやライブラリの機能を理解してうまく使いこなして自前で実装するコードを減らすスキルはとても重要です。
この投稿のAfterのコードは誰がどう読んでもなんの面白みもヒネリもない平凡すぎるコードなんですが、チームメンバーから見るとそんなコードが一番尊いのではないでしょうか。
なお、敢えてツッコミどころを探すなら、User.find_by(email: email)find_by! にしておかないとヌルポの可能性がありそうですね。

imaz さんには、回らないお寿司券として使えるギフト券をプレゼントいたします。

審査員特別賞

f:id:grooves:20160202195746p:plain:w500

松田賞

joker1007 さんの パラメーター引数のクラス化とActiveSupport::Concernによるモジュール化

Before

# app/models/pasokara.rb
class Pasokara < ActiveRecord::Base
  validates_presence_of :title, :fullpath, :md5_hash
  validates_uniqueness_of :md5_hash

  include SimpleTaggable

  searchable do
    text :title, stored: true
    string :title_sort do
      title
    end
    string :tags, multiple: true, stored: true do
      tags.map(&:name)
    end
    string :nico_vid, stored: true
    integer :nico_view_count, trie: true
    integer :nico_mylist_count, trie: true
    text :nico_description, stored: true
    time :nico_posted_at, trie: true
    integer :duration, trie: true
  end

  class << self
    def all_with_facet_tags(page: 1, per_page: 100)
      search(include: [:tags]) do
        facet :tags
        paginate page: page, per_page: per_page
      end
    end
  end

  paginates_per 100

  include CreateMethods
  
  # 省略...
end

After

# app/models/pasokara.rb
class Pasokara < ActiveRecord::Base
  validates_presence_of :title, :fullpath, :md5_hash
  validates_uniqueness_of :md5_hash

  include SimpleTaggable
  include Searching

  paginates_per 100

  include CreateMethods

  # 省略...
end

# app/models/pasokara/searching.rb
module Pasokara::Searching
  extend ActiveSupport::Concern

  included do
    searchable do
      text :title, stored: true
      string :title_sort do
        title
      end
      string :tags, multiple: true, stored: true do
        tags.map(&:name)
      end
      string :nico_vid, stored: true
      integer :nico_view_count, trie: true
      integer :nico_mylist_count, trie: true
      text :nico_description, stored: true
      time :nico_posted_at, trie: true
      integer :duration, trie: true
    end
  end

  class SearchParameter
    attr_reader :keyword, :tags, :page, :per_page
    def initialize(keyword: nil, tags: nil, page: 1, per_page: 100)
      @keyword = keyword
      @tags = tags || []
      @page = page || 1
      @per_page = per_page.to_i
      freeze
    end
  end

  module ClassMethods
    def search_with_facet_tags(search_parameter)
      search(include: [:tags]) do
        fulltext search_parameter.keyword if search_parameter.keyword.present?
        search_parameter.tags.each do |tag_name|
          with(:tags, tag_name)
        end
        facet :tags
        paginate page: search_parameter.page, per_page: search_parameter.per_page
      end
    end
  end
end

松田氏 コメント

何か特別すごいテクニックが使われているわけでもないんですが、生々しくて実践的な感じがよいですね。
Searchingというモジュール名は、「検索」という意味だろうとは思いますが、現在進行形だと「探してる」みたいなニュアンスになってしまって違和感があります。
Search(検索)、Searcher(検索するやつ)、Sarchable(検索できるやつ)ぐらいでしょうか。

和田賞、藤村賞

kenchan さんの【Refactor Me】if else のテストを書いてリファクタリング

before

0 <= age or raise

if 0 <= age && age <= 2
  'baby'
elsif 3 <= age && age <= 6
  'little child'
elsif 7 <= age && age <= 12
  'child'
elsif 13 <= age && age <= 18
  'youth'
else
  'adult'
end

after

require 'test/unit'

class TimeOfLife
  attr_reader :label, :min_age, :max_age

  def initialize(label, min_age, max_age)
    @label, @min_age, @max_age = label, min_age, max_age
  end

  def include?(age)
   (@min_age..@max_age) === age
  end
end

TIME_OF_LIFES = [
  TimeOfLife.new('baby', 0, 2),
  TimeOfLife.new('little child', 3, 6),
  TimeOfLife.new('child', 7, 12),
  TimeOfLife.new('youth', 13, 18),
  TimeOfLife.new('adult', 19, Float::INFINITY)
].freeze

def age_to_label(age)
  tol = TIME_OF_LIFES.find {|t| t.include?(age) }

  tol&.label || raise
end

class MyTest < Test::Unit::TestCase
  def test_negative_age
    assert_raise do
      age_to_label(-1)
    end
  end

  def test_baby_min
    assert_equal('baby', age_to_label(0))
  end

  def test_baby_max
    assert_equal('baby', age_to_label(2))
  end

  def test_little_child_min
    assert_equal('little child', age_to_label(3))
  end

  def test_little_child_max
    assert_equal('little child', age_to_label(6))
  end

  def test_child_min
    assert_equal('child', age_to_label(7))
  end

  def test_child_max
    assert_equal('child', age_to_label(12))
  end

  def test_youth_min
    assert_equal('youth', age_to_label(13))
  end

  def test_youth_max
    assert_equal('youth', age_to_label(18))
  end

  def test_adult
    assert_equal('adult', age_to_label(19))
  end

  def test_immortality
    assert_equal('adult', age_to_label(200))
  end

  def test_float
    assert_equal('baby', age_to_label(0.1))
  end
end

和田氏 コメント

この投稿が良かったのは、レガシーコードのリファクタリングの過程を公開している点です。
ここで言う「レガシーコード」とは、テストコードが書かれていないコードのことです。
https://github.com/kenchan/forkwell-jobs-sushi-contest-2015

リファクタリングは、小さく安全なステップで対象のコードを段階的に綺麗にしていくプロセスです。
安全なステップの「安全」を支えるのはテストです。
そして、レガシーコードはテストが書かれてこなかった故に、テストが書きにくい、
あるいはそのままではテストが書けない構造になっていることが多々あります。

つまりリファクタリングにはテストが必要だが、テストを書くためにはリファクタリング(構造変更)が必要、という一種のデッドロックに陥ります。このデッドロックに直面したとき、私たちは

  • テストを書けるようになるまでテスト無しで構造変更するか
  • 構造変更を伴わず強引に大外からテストする

という二つの道のどちらかを選択します。 kenchan さんの選んだ道は前者で、
それは対象コードが十分に小さいから、という判断も働いたのでしょう。
github に公開されている履歴を辿っていくと

  1. テストが書けない構造をテスト可能にするために最小限の変更を行う
  2. 小さなテスト基盤を整えてテストを必要なだけ記述
  3. そのテストを通しながら内部を改善する
  4. 安全圏に達したら、必要に応じて段階的に対象コードの(外部も含め)構造変更を行う

というレガシーコード改善の王道とも言える進め方をしている点が印象に残りました。

藤村氏 コメント

ベタ書きだった処理が、適切な範囲の責務を持つクラスに切り分けられていく。
小さな例ですがオブジェクト指向リファクタリングのあるべき姿を示していると感じました!
それでいて冗長になっていないのが個人的に非常に好みです。

僕だったら{ Range: Label }Hashをlookupして終わりにしちゃうところですが、このくらい簡潔に書けるならクラスにしてもいいなあと。
指摘するとしたら、TimeOfLifeはLifeStageのほうが通りがよさそう。

松田氏 コメント

※同氏はこちらのコードを次点としておりましたが、その選出時のコメントを掲載しています。

1つぐらいはRubyの新機能を活用したものを採りたかったのでこれを選びました。
この内容ならクラスを作るまでもないだろうという意見もありそうですが、そのあたりは敢えてそういう設計を選んだようですし、そういった方向性も含めて、全体的な仕事の丁寧さに好感が持てます。
コミットログをしたためてくれたのは素晴らしいんですが、日本語コミットはちょっと読む気しないのでプラマイゼロ。
lifeの複数形はlivesなような気がするのでそのあたりが減点対象で、そもそもそういう紛れがありそうな名前を避けるのもスキルのうちかもしれません。

特別賞

今回、Ruby賞、審査員賞それぞれで審査員の意見が一致しましたので、特別にもう一つ賞をご用意しました。
特別賞に選ばれたのは、teruki.shinohara さんの ブロックを使って、繰り返し使うコードをメソッドに切り出すです。

※商品は審査員特別賞と同じものを贈呈いたします。

before

# fbのfeedを全て取得する
def get_fb_feeds(fb_api_client)
  option = {iam: 'fb'}
  response = fb_api_client.get_feeds(option)
  fb_feeds = response

  until response.empty?
    option[:max_id] = response.last.id
    response = fb_api_client.get_feeds(option)
    fb_feeds += response
  end

  fb_feeds.flatten
end

# twのtweetを全て取得する
def get_tw_tweets(tw_api_client)
  option = {iam: 'tw'}
  response = tw_api_client.get_tweets(option)
  tw_tweets = response

  until response.empty?
    option[:from_id] = response.last.id
    response = tw_api_client.get_tweets(option)
    tw_tweets += response
  end

  tw_tweets.flatten
end

after

def paginate(&block)
  response = yield(nil)
  sns_posts = response

  until response.empty?
    response = yield(response)
    sns_posts += response
  end

  sns_posts.flatten
end

# fbのfeedを全て取得する
def get_fb_feeds(fb_api_client)
  option = {iam: 'fb'}
  paginate do |response|
    option[:max_id] = response.last.id unless response.nil?
    fb_api_client.get_feeds(option)
  end
end

# twのtweetを全て取得する
def get_tw_tweets(tw_api_client)
  option = {iam: 'tw'}
  paginate do |response|
    option[:from_id] = response.last.id unless response.nil?
    tw_api_client.get_tweets(option)
  end
end

藤村氏 コメント

僕も同じようなコードを半年前に書いたので懐かしい思いで見させて頂きました。繰り返し処理の抽象化により、コードがあるべき方向に進んでいると感じました。
paginate の実態は繰り返し処理をして値を返すメソッドなので、paginate(ページを付ける)だとミスマッチかも。
APIリクエストを発行する Client をデータソースとする Enumerator を定義して、それによるストリーム処理にするとよさそうです。

例えば SnsPostsEnumerator.new(FacebookClient.new).each.all という風に書けると、馴染みがよくなりそうです。
オーバーデザインだ!と言われる気もしますが。

プログラムはチームで開発している事が多いので、このように議論のスタートになるリファクタリングはとても価値が高いと個人的には思っています。コードレビューを通して完成形に近づけばよいので。

joker1007 さん、kenchan さん、teruki.shinohara さんには、ネスカフェコーヒーメーカーをプレゼントします。

参加賞

f:id:grooves:20160202195828p:plain:w500

コードをきれいにするアレを手に入れたのはこちらの3名です

  • hanachin_
  • yancya
  • 5t111111

すべての受賞者には、Forkwellより個別にご連絡させていただきます。

多くのご参加、誠にありがとうございました!


Forkwell Jobs では、IT 開発者が主役となって活躍できる会社を増やし、そんな会社と開発者が出会うきっかけを創造したいと考えています。
そのために、開発環境をしっかりと整備し、公開できる会社の求人だけを厳選して掲載しています。

Ruby が(きちんと)使える環境に転職したいとお考えの方は、ぜひ Forkwell Jobs をご活用ください!

jobs.forkwell.com

Rails で cancancan と action_args の2つの gem を共存して使う方法

こんにちは、Forkwell 事業部の正徳です。

タイトルにもあるように、Forkwell Jobs の開発では cancancanaction_args の2つの gem を使用しています。この2つの gem を一緒に使う際に問題が起きましたので「問題の紹介」と「解決するコード」を紹介したいと思います。

各 gem の簡単な紹介

知らない方もいると思いますので、各 gem の概要を書いておきます。

cancancan

コードの各所に散らばりがちな権限を Ability という1つのファイルで管理できるようになります。

# app/models/ability.rb
class Ability
  include CanCan::Ability

  def initialize(user)
    user ||= User.new # guest user (not logged in)
    if user.admin?
      can :manage, :all
    else
      can :read, :all
    end
  end
end

# app/controllers/users_controller.rb
class UsersController < ApplicationController
  load_and_authorize_resource

  def create
    if @user.save
      # do something
    else
      render :new
    end
  end

  private

  def user_params
    params.require(:user).permit(:name, :age)
  end
end

action_args

コントローラーで使用する値を引数に定義することで、 params を使わずにコードを書けるようになります。

# app/controllers/users_controller.rb
class UsersController < ApplicationController
  # white-lists User model's attributes
  permits :name, :age

  # the given `user` parameter would be automatically permitted by ActionArgs
  def create(user)
    @user = User.new(user)
  end
end

2つの gem を使うときの問題点

前述したコードからも分かるように、それぞれの gem で Strong Parameters に対応する方法が異なります。このため、両方のいいとこ取りをするような下記のコードはエラーになってしまいます。

class UsersController < ApplicationController
  load_and_authorize_resource

  permits :name, :age

  def create
    if @user.save
      # do something
    else
      render :new
    end
  end

  def update(user)
    if @user.update(user)
      # do something
    else
      render :edit
    end
  end
end

こんなコードが書けるようにしたいですよね!というわけで、共存させるようなコードを書きました。

コード

# refs
# https://github.com/CanCanCommunity/cancancan/blob/v1.13.1/lib/cancan/controller_resource.rb#L79
# https://github.com/asakusarb/action_args/blob/v2.0.0/lib/action_args/params_handler.rb#L45
module CanCan
  module ActionArgsEx
    def permits(*attributes)
      super
      UsingActionArgs.include_once(self)
    end
  end

  module UsingActionArgs
    def self.included(klass)
      klass.singleton_class.prepend(ActionArgsEx)
    end

    def self.include_once(klass)
      method_name = "#{klass.controller_name.singularize}_params".to_sym
      return if respond_to?(method_name)

      m = Module.new do
        define_method method_name do
          model_name = klass.instance_variable_get(:@permitting_model_name)
          permitted_attributes = klass.instance_variable_get(:@permitted_attributes)
          key = (model_name || klass.controller_name).singularize.underscore.to_sym
          params[key].permit(*permitted_attributes)
        end
      end
      klass.include(m)
    end
  end
end

Ruby のメタプログラミング力が上がりそうなコードですね!この Module がやっている概要は下記の通りです。

  1. include したクラスの self.permits を上書きする
  2. permits が呼ばれたら、一度だけ 3. 以降の処理を実行する
  3. コントローラーのクラス名から xxx_params というメソッド名を作る
  4. xxx_params のメソッドを持つ Module を動的に生成し、コントローラーで include する

これにより、 load_and_authorize_resourcexxx_params のメソッドを呼び出すことができるため、エラーが起きなくなります。

ちなみに、使い方はコントローラーで include するだけです。

class UsersController < ApplicationController
  include CanCan::UsingActionArgs

  load_and_authorize_resource

  permits :name, :age

  def create
    if @user.save
      # do something
    else
      render :new
    end
  end

  def update(user)
    if @user.update(user)
      # do something
    else
      render :edit
    end
  end
end

cancancan と action_args という2つの gem を組み合わせて使う際の解決方法をご紹介しましたが、いかがだったでしょうか?

このコードが cancancan と action_args を両方とも使っている Rails プロジェクトの役に立てば幸いです。

【RubyKaigi応援企画】リファクタリングコンテスト期間延長のお知らせ & お題 第2弾を用意しました

明日から待ちに待った RubyKaigi ですね!

f:id:app2641:20151210150536p:plain

Forkwell Jobs では RubyKaigi 2015 応援企画 として回らないお寿司が食べられる リファクタリングコンテスト を開催しています。

https://jobs.forkwell.com/campaigns/rubykaigi-2015

前回の投稿 では、投稿するネタが見つからないという方に向けてリファクタリングのお題をご用意させて頂きました。

このお題をきっかけに、投稿が少しずつ増えてきましたので、
それならばと、さらに五つのお題をご用意しました。

今回も曲者揃いのコード達です。
優秀作品に選ばれる、それはすなわち豪華審査員があなたのリファクタリングスキルにお墨付きを与えたようなもの!
是非挑戦してみてください。

重要なお知らせ

さて、ここでリファクタリングコンテストに関してお知らせがあります。
タイトルにもあるように RubyKaigi 2015 応援企画と銘打って開催してきたリファクタリングコンテストを年末まで期間を延長することが決定致しました。

変更後の募集締め切り: 2015 年 12 月 31 日

皆様からの投稿が社内でも好評で(期間内に投稿いただきありがとうございます!)、このまま終わらせてしまうのはどうにも勿体無い!という想いからです。
Forkwell Jobs は RubyKaigi だけでなく Ruby のコミュニティ全体を応援しているんですから少しくらい良いですよね?

リファクタリングの投稿は 2015 年 12 月 31 日いっぱいまで受け付けています。
ひとり何回でも投稿可能です。どんどんご応募ください。
年末といえば大掃除。一年の間で汚れてしまったあなたのそのコードもリファクタリングして綺麗さっぱりにして新しい年を迎えてみませんか?
そのままリファクタリングコンテストに投稿して美味しいお寿司(あるいはあのお肉やあの蟹やあのディナーなどなど)をゲットしましょうー。

ご応募お待ちしております!

【RubyKaigi応援企画】リファクタリングコンテストのお題を用意しました

f:id:yachibit:20151126100707p:plain

以前お知らせしたとおり、Forkwell Jobs では、RubyKaigi 2015 応援企画として「リファクタリングコンテスト」を開催しています。

https://jobs.forkwell.com/campaigns/rubykaigi-2015

リリース後、印象や使い勝手について知人にヒアリングしてみたところ、

  • 投稿するネタが見つからない
  • リファクタリングのネタを探すのが大変

などのご意見をいただきました。

そこで今回は、そういった方のために運営側で before コードをご用意しました。
before コードをもとにあなたのリファクタリングを投稿しましょう!

この before コードをリファクタリングしてくれた方にも寿司のチャンスがあります!
またご存知ない方もいると思うのですが、Ruby 賞の商品、実は寿司だけではなく、うなぎや焼き肉券として使えます。

年末の大掃除としてコードをきれいにしてみませんか?皆様の応募をお待ちしています!

非エンジニアメンバーへのプログラミング研修のススメ

f:id:grooves:20151203114405j:plain

弊社のエンジニアチームでは、エンジニア以外のメンバーとの相互理解深めるための取り組みとして、非エンジニアメンバーを対象とした Ruby研修を行っています。

営業、総務・人事、カスタマーサポートなどの異なる職種のメンバーと一緒に、 環境構築から簡単なプログラミングまでを社内のエンジニアが教え合っています。

当然、Ruby は rbenv を使って最新版をインストール。(講師と環境が異なると、意図しないところでつまづくことが増えるため)

先週の研修に初めて参加したメンバーは、Ruby で九九ができるようになりました! と喜んでいました。

受講者の中には、gemを使って、Yahoo!ファイナンスから売上、経常、利益率などをスクレイピングしてきて 売上の伸び率から調子の良い企業にあたりをつけ、優先的に営業活動をするようになったメンバーも出てきました。

営業メンバーが作った 気になるIT系上場企業の業績バブルチャート

営業メンバーが書いたコード

#yahooファイナンスから証券コードで指定した上場企業の売上/営利/経常/純利をスクレイピングしてくるコード

require 'mechanize'

agent = Mechanize.new

comp_code = [9984, 9449, 6095, 6094, 6084, 6075, 6069, 6054, 4841, 4835, 4829, 4824, 4819, 4784, 4777, 4772, 4766, 4765, 4755, 4751, 4747, 4726, 4689, 4309, 4308, 4305, 4304, 4293, 4288, 4281, 3845, 3843, 3834, 3828, 3815, 3793, 3789, 3788, 3786, 3783, 3778, 3775, 3774, 3772, 3770, 3768, 3765, 3754, 3739, 3730, 3722, 3715, 3696, 3695, 3688, 3685, 3679, 3675, 3674, 3668, 3665, 3664, 3662, 3660, 3659, 3656, 3647, 3646, 3645, 3641, 3639, 3634, 3632, 3629, 3627, 3624, 3622, 3328, 3319, 3195, 3182, 3179, 3092, 3090, 3071, 3060, 2656, 2497, 2495, 2492, 2491, 2489, 2484, 2477, 2461, 2454, 2450, 2440, 2432, 2413, 2389, 2379, 2371, 2356, 2351, 2193, 2177, 2159, 2155, 2148, 2138, 2132, 2130, 2122, 2121, 2120]

comp_code.each do |x|
  
  puts "#{x}"

  page = agent.get("http://profile.yahoo.co.jp/independent/#{x}")

  h1_result = page.search('h1')

  p h1_result.text().strip()

  table_tr = page.search('table.yjMt tr')

  puts table_tr[5].text()
  puts table_tr[6].text()
  puts table_tr[7].text()
  puts table_tr[8].text()
   
  puts
  puts

  sleep(1) #善意
  
end

ビジネスサイドのやつらがなかなかわかってくれないといった声はよく聞きますが、 こちらから歩み寄ってみると、意外とわかりあえることもある、と実感しております。

皆様の職場でもぜひ試してみてください。

告知

groovesが運営する Forkwell Jobs では、ただいま RubyKaigi 2015 応援企画として「リファクタリングコンテスト」を開催しています。

f:id:yachibit:20151126100707p:plain https://jobs.forkwell.com/campaigns/rubykaigi-2015

皆さま自身が行ったリファクタリングの Ruby コードの Before と After を投稿してください。 豪華審査員による厳正な審査によって選ばれた方にはなんと、回らない寿司にご招待します!

さらに参加特典もご用意。「いいね!」で投票してくれた方の中から抽選で、 コードをキレイにする「アレ」をプレゼントしちゃいます! 良いリファクタリング作品にはぜひ「いいね!」を押してみてくださいね。