チームのstyleguideを作ろう! #2

Product Develop Division Renosyチームの渡邉(@evitch)と申します。半年前にGA technologiesに転職し、現在はRailsエンジニアとしてRENOSYプロダクトのフォーム開発を担当しています。Ruby歴はまだ3ヶ月です。

先週(Renosyチーム社内勉強会を実施しました - 2019/07/05)に続き、Renosyチームで勉強会を実施しましたので、その内容をアウトプットしていきたいと思います。

Renosyチームでstyleguideを作っている目的を再掲します。

テーマ: チーム内のstyleguideを作ろう!

Renosyチームは、結成当初は3人のエンジニア体制でしたが、現在10名まで増え、そのバックボーンも新卒からベテラン中途まで様々です。 そのため、コードの品質やチーム開発のためにstyleguideを作った方がいいなと感じることが増えてきました。 しかし、白紙な状態からつくるのも難しいため、まず最初はCookpadさんのstyleguideを参考にさせていただこうと考えました。 そこで、勉強会の時間を使って、Cookpadさんのstyleguideを読みあって、議論を深め、自分たちのstyleguideへエンハンスしていきたいと考えています。

github.com

今週取り扱ったテーマは以下の2つです。

  • 制御構造
  • メソッド呼び出し

制御構造

制御構造については熱い議論が繰り広げられ、このお題だけで30分以上も時間を使ってしまいました。 知らなかったのですが、Rubyでよくある nil? のような真偽値を返すメソッドのことを述語メソッドというらしいです。おしゃれですね。 以下、決定事項を列挙します。

  • unless は後置の場合のみOKとする。

  • while !condition の代わりに until condition と書くこと

  • ifunless、および case の条件式の後ろは改行すること、同じ行に本体コードを続けて書いてはならない

  • ifunless、および casethen: を使用してはならない

  • while および until の条件式の後ろは改行すること。同じ行に本体コードを続けて書いてはならない

  • while および untildo: を使用してはならない

  • unless および until の条件式に複数の項を || で結合した論理式 (加法標準形) を書いてはならない

  • 条件式が長く複数行になってしまう場合は、その条件式を述語メソッドとして抽出し、適切な名前を付けて使用すること

  • 制御構造が代入の右辺に来る場合は、本体コードのインデントを2レベル下げ、制御構造の終わりの end 行のインデントを本体コードより1レベル戻すこと

メソッド呼び出し

どのような場合にブロックを do/end で書くか、中括弧で書くかは議論が白熱しました。1行の場合は中括弧で書くというのは全員が納得でしたが、複数行の場合でもメソッドチェインする時は中括弧を使った方がいいという意見もあがりました。 以下、決定事項を列挙します。

  • メソッド呼び出しの括弧は、省略を許可された場合を除いて、省略してはならない

  • DSL-like メソッドの呼び出しでは、メソッド呼び出しの括弧を省略して良い。ただし、第一引数全体が括弧で囲まれる場合は警告が出てしまうため省略してはならない。DSL-like メソッドとは以下のメソッドである。

    • p, print, puts, require などのKarnelモジュールのメソッド
    • attr_reader や private など、クラスやモジュールの定義で用いる宣言的メソッド (Rails のようなフレームワークが提供しているものも含む)
    • redirect_to や render などの ActionController が提供する、アクションに対する継続処理を宣言するためのメソッド
    • その他、DSL のために用意されていると考えられるメソッド
  • 引数なしのメソッド呼び出しは括弧を省略すること

  • メソッド名とメソッド呼び出しの括弧の間に空白を入れてはならない

  • パラメータリストの末尾にハッシュリテラルを書く場合は、ハッシュリテラルの括弧を省略すること

# good
foo(1, 2, foo: :bar, baz: 42)

# bad
foo(1, 2, { foo: :bar, baz: 42 })
  • ブロック付きメソッド呼び出しを1行で書く場合は、ブロックを中括弧で書くこと

  • do/end によるブロックでは、doの前後に空白を1つ入れ、ブロックパラメータの後で改行し、end は独立した行に書くこと。ブロック本体のインデントは1レベル下げ、end のインデントはメソッド呼び出しの1行目にあわせること。

# good
[1, 2, 3].each do |num|
  puts num
end

# bad
[1, 2, 3].each do |num|
    puts num
  end

# bad
[1, 2, 3].each do |num|
                 puts num
               end

# bad
[1, 2, 3].each do |num| puts num end
  • 以下の条件を満たす場合は、&とシンボルを使ってブロック内のメソッド呼び出しを簡潔に記述することを推奨する*1
    1. ブロック引数が1つのみ
    2. ブロックの中で呼び出すメソッドには引数がない
    3. ブロックの中では、ブロック引数に対してメソッドを1回呼び出す以外の処理がない
# bad
.map { |_| _.id }

# good
.map(&:id)

感想

「コードレビューの時に差分がわかりやすい」とか「rails cにコピペしても動く」とか、自分では考えもしないような意見が出てくるので、毎回とても楽しみに参加しています。このような互いを高め合えるような取り組みは貴重なので、これからも続けていきたいと思います。

*1:伊藤 淳一(2017)『プロを目指す人のためのRuby入門—言語仕様からテスト駆動開発・デバッグ技法まで』 技術評論社.