株式会社クリアコード > ククログ

ククログ


インターンシップで学んだこと1:コメントを書きたくなるときはコードを見直す機会

RubyKaigi 2013で発表したらインターンシップの応募があり、6/17から7/29まで1ヶ月半くらい週3日でインターンシップを実施しました。インターンシップでやったことはメモに残して公開していました。ただメモに残すだけではなく、まとめておき、次のインターンシップや新しくクリアコードのメンバーになった人に伝えるために役立てようと試みています。

クリアコードではインターンシップをクリアコード側もいろいろ学ぶ機会として捉えています。今回のインターンシップでクリアコードが得られたものは以下のようなものです。やった価値がありました。

  • 自分たちがクリアなコードを書くために大事にしていることのいくつかを明文化できた。
  • こうすれば伝わりやすいのではないかと考えた方法のうち、効果があるものとないものがわかった。
  • 新しい人とコミュニケーションをとりやすくする方法を試して、効果があることがわかった。

これらは大きな項目なので、少しずつもう少し細かい単位でまとめていきます。今回まとめるのは、明文化できたクリアなコードを書くために大事にしていることの1つです。

背景

インターンシップ1日目では、作ろうとしているWebアプリケーションが技術的に簡単に実現できそうかどうかを試しました。具体的にいうと、コアとなる機能を使ったコマンドラインツールを作りました。コマンドラインツールを作ることで簡単に実現できそうだということがわかりました。

作ったコマンドラインツールを見ると気になるところがあったので、インターンシップ2日目は気になるところを説明しながら整理しました。その中で明文化できたクリアなコードを書くために大事にしていることの1つが「コメントを書きたくなるときはコードを見直す機会とすること」でした。

コードの中のコメント

クリアコードの人が書くプログラムにはコメントが少ないです。これは、「コメントで書いていることを本当にコードで表現できないか」を考えてコードを書いているからです。多くの場合はコードで表現することができるので、コメントを書くことが少なくなります。

コメントが少ないからといって「絶対コメントを書くな!」と思っているわけではありません。コメントを書くことでコードだけよりも理解が進むと判断したらコメントを書きます。

例えば、groongaで高速な位置情報検索をしているコードには以下のようなコメントを書いています。リンク先の記事で「例なので、実際の処理よりも大雑把に説明します。」と書いていることの「実際の処理」の部分です。

groonga/lib/geo.c:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
/*
  b: base_point
  x: geo_base
  0-83: sub meshes. 0-83 are added order.

j: -5  -4  -3  -2  -1   0   1   2   3   4
  +---+---+---+---+---+---+---+---+---+---+
  |74 |75 |76 |77 |78 |79 |80 |81 |82 |83 | 4
  +---+---+---+---+---+---+---+---+---+---+
  |64 |65 |66 |67 |68 |69 |70 |71 |72 |73 | 3
  +---+---+---+---+---+---+---+---+---+---+
  |54 |55 |56 |57 |58 |59 |60 |61 |62 |63 | 2
  +---+---+---+---+---+---+---+---+---+---+
  |48 |49 |50 |  b    |       |51 |52 |53 | 1
  +---+---+---+       |       +---+---+---+
  |42 |43 |44 |       |x      |45 |46 |47 | 0
  +---+---+---+-------+-------+---+---+---+
  |36 |37 |38 |       |       |39 |40 |41 | -1
  +---+---+---+  base meshes  +---+---+---+
  |30 |31 |32 |       |       |33 |34 |35 | -2
  +---+---+---+---+---+---+---+---+---+---+
  |20 |21 |22 |23 |24 |25 |26 |27 |28 |29 | -3
  +---+---+---+---+---+---+---+---+---+---+
  |10 |11 |12 |13 |14 |15 |16 |17 |18 |19 | -4
  +---+---+---+---+---+---+---+---+---+---+
  | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | -5
  +---+---+---+---+---+---+---+---+---+---+
                                            i
*/

1日目に書いたスクリプトにはいくつかコメントがありました。これを見て、「自分たちはコメントを書くときにこのコメントは本当にコードで表現できないかを考えているなぁ」と気づきました。ということで、一緒にコメントを1つずつ確認し、コードで表現できないか考えることにしました。

コードと同じ内容のコメントか

最初に確認したコメントは以下のコメントです。

1
2
3
4
5
def show_html_content(io)
  # Nokogiri でパースし、内部テキストを出力する
  content = Nokogiri::HTML(io)
  puts content.text
end

このコメントはメソッド内の処理していることそのものについて書いています。つまり、コードの内容とコメントの内容が同じです。コードと同じ内容のコメントは削除します。

1
2
3
4
def show_html_content(io)
  content = Nokogiri::HTML(io)
  puts content.text
end

削除する理由は以下のとおりです。

  • 重複した情報なので必要ないから。
  • 「コメントに書くくらいだから何か特別な情報が入っているのではないか?」と読む人に勘違いさせないため。

後者の「読む人に勘違いさせないため」ということを少し補足します。「書いた人は読む人の助けとなるようにコードもコメントも書いているはず」という前提にたって読んでいる人は、本当は意味のないコメントにも「一見意味がなさそうだが、実はなにか意味があるのではないか?」という視点で読みます。そのため、「本当は必要のないコメント」を「必要なコメントではないか?」と勘違いしてしまい、コメントの必要性を探ってしまいます。そして、読んだ後に「なんだ、必要のないコメントだったのか」と気づきます。

参考:

処理の塊を説明しているコメントはメソッド名で表現する

次に検討したコメントは以下のコメントです。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
def open_epub(filename)
  uri_array = Array.new
  epub_book = EPUB::Parser.parse(filename)
  metadata = epub_book.metadata

  # xhtml ドキュメントがどの順番で出てくるか記録する
  epub_book.each_page_on_spine do |item|
    if item.media_type == "application/xhtml+xml"
      uri = item.href
      uri_array << uri.to_s
    end
  end
  # ... 
end

このコメントは処理の塊で何をしているかを説明しています。コメントの内容の方が少し抽象度の高い説明になっており、コードと同じ内容というわけではありません。このように処理の塊に抽象度の高い説明をつけている場合は処理をメソッドに切り出してコメントで表現していたことをメソッド名で表現します。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
def extract_xhtml_uri_array(epub_book)
  uri_array = Array.new

  epub_book.each_page_on_spine do |item|
    if item.media_type == "application/xhtml+xml"
      uri = item.href
      uri_array << uri.to_s
    end
  end

  uri_array
end

def open_epub(filename)
  epub_book = EPUB::Parser.parse(filename)
  metadata = epub_book.metadata

  uri_array = extract_xhtml_uri_array(epub_book)
  # ... 
end

「どの順番で出てくるか」という情報が失われているので、コメントの情報をすべてをメソッド名で表現できていませんが、処理の塊にコメントで説明していた情報をメソッド名で表現するということはこういうことです。

まとめ

インターンシップでクリアコードが学んだことをまとめはじめました。今回は、自分たちがクリアなコードを書くために「コメントを書きたくなるときはコードを見直す機会」と捉えていることについてまとめました。

今回でてきたコードはコメントにだけ着目したもので、その後、他のところも整理されよりクリアなコードになっていきました。気になる人はranguba/epub-searcherを確認してみてください。

2013-08-05

インターンシップで学んだこと2:1人で開発しているときはていねいに開発を進める

インターンシップ1日目と2日目ではコメントに注目するとよいということを学びました。インターンシップ3日目のメモを読み返すと3日目は3つ学んだことがありました。今回はそのうちの1つ「1人で開発しているときにtypoとどうつきあっていくか」です。

背景

インターンシップは基本的にメンターがいつもインターンのそばにいて一緒に開発を進めていく予定でしたが、打ち合わせなど他の予定が入ってしまうこともありました。そのようなときはインターンが1人で開発を進めていました。メンターは他の予定から戻ってきたときに「どうだった?」「詰まっているところはない?」などと聞きながらコミットメールを読んでいました。

コミットメールを読めばだいたいどんなことをやっていたかはわかりますが、「変なことをしていないか?」という視点で読んでいるわけではないので細かいミスすべてに気づくわけではありません。どちらかというと時間を越えてペアプログラミングしている気分で読んでいます。これは、コードレビューでの読み方とは違う読み方です。

そんなわけで、2日目に追加したテスト名にtypoがあり、テストとして認識されていないことに気づいていませんでした*1。typoを直してテストとして認識されるようにしたところテストが失敗していました。せっかくテストを追加しながら開発していたのですが活かせていませんでした。

typoはよくあることですし、すぐに直せることなので、typoしてしまうことが悪いことだとは思いません。typoを防ぐことよりもtypoしてもすぐに気づいて直せるようにした方が現実的でしょう。ということで、「1人で開発しているときにtypoとどうつきあっていくか」についてです。どうつきあっていくかを最初に言うと、ていねいに開発を進めてつきあっていく、です。

開発を継続するためのテスト

開発するときはテストを書きますが、それは開発を駆動するためではないのでテスト駆動開発という言葉を使うことはあまりありません。「テストを書きながら開発をするやり方」をざっくりと説明するために便利なのでテスト駆動開発という言葉を使うくらいです。テストを書くのは開発を駆動するためではなく、開発を継続するためです。

いくつもフリーソフトウェアの開発に関わっていると、数カ月ぶりにコミットするということがあります。そんなときはコードの隅々まで覚えているという状態ではありません。そんな状態でも安心してコードを変更できるようにするためにテストを書いておきます。他にも、開発を長く続けていると新しいプラットフォームや新しいライブラリーに対応したりする場合にテストがあると安心してコードを変更できます。つまり、いつまでも安心してコードを変更していけるように、開発を継続していけるように、そのためにテストを書いています。

開発を駆動するためにテストを書いているわけではないので、テストファーストで開発を進めるときもあればそうでない場合もあります。入力のパターンがわかっている場合は1つずつテストファーストで開発を進めていきますが、さぐりさぐりで開発しているときはある程度実装が見えてきてからテストを書きます。

このように、あまりテスト駆動開発を取り入れていませんが、テスト駆動開発の中にていねいに開発を進めるためによいやり方があるので、それは取り入れています。

最初にテストを失敗させる

テスト駆動開発の中にあるていねいに開発を進めるためのよいやり方とは「最初にテストが失敗させ、ちゃんとテストが実行されていることを確認する」やり方です。これは、1人で開発しているときでもなるべく早くtypoに気づけるようになるからです。具体的にどうやっているかというと、最初は期待する値として"XXX"という必ず失敗する値を指定して実行しています。別に"XXX"でなくてもよいのですが、コードのコメントの中で「なんか気をつけて!」くらいの意味合いで「XXX」が使われるので、失敗する値としても"XXX"を使っています。

1
2
3
def test_plus
  assert_equal("XXX", 1 + 2)
end

このコードをコミットすることはないのでこのように開発していることを知っている人はほとんどいないでしょう。インターンシップのときも伝え忘れました。単に、「テストを書いたら最初に失敗させてテストが実行されていて確認するといいよ」と伝えたくらいだった気がします。

テストでは補完機能を使わない

typoを早く見つけるために気をつけていることがもう1つありました。それは、テストコードの方ではあまりエディターの補完機能を使わないということです。テストコード固有のところでは使いますが、テスト対象のコードを書くとき(開発した機能をテストの中で使っているとき)には使いません。これは、補完機能を使ってしまうと開発しているコードの中にあるtypoをそのままテストでも使ってしまうからです。そのまま使ってしまうとtypoに気づくことができません。

テストコードの中では開発しているコードを使う側の気持ちになって書きたいので、エディターの補完機能に頼らず実際に入力しています。

このやり方もインターンシップ中には伝え忘れた気がします。

まとめ

メインで開発している人が自分だけの場合は、typoに気づきづらいものですが、ていねいに開発を進めれば早いうちに気づけるものですよ、ということをまとめました。普段、意識せずにやっていて、インターンシップのときもそれほどまとまった形で伝えられなかったことが、こうしてインターンシップのメモをまとめ直すことでまとまった形にできてよかったです。

*1 こういうときはdef test_XXXではなく、test "..." {}でテストを定義したほうがよいのではないかという気がしてきます。

2013-08-13

パーフェクトRuby

2013年8月にRuby 2.0.0に対応したRubyの解説書が技術評論社から出版されました。

パーフェクトRuby (PERFECT SERIES 6)
Rubyサポーターズ/すがわら まさのり/寺田 玄太郎/三村 益隆/近藤 宇智朗/橋立 友宏/関口 亮一
技術評論社
(no price)

この本の特徴はなんといっても網羅性でしょう。600ページを超える紙面でRubyの基本的な機能から、2.0.0で新しく導入された機能、さらによく使われる標準添付のライプラリーとgemの解説をしています。長いことRubyを使っていますが、以下の機能はこの本を読むまで知りませんでした*1

  • #keep_if
  • Enumerator#feed
  • Tread#fork
  • MethodTransplaning
  • Method#parameters
  • Module#method_undefind

機能の説明にはサンプルコードと実行結果がついています*2。文章だけの説明でピンとこない場合でも、コードで理解できます。この本の一番優れているところはここでしょう。

サンプルコードと実行結果がついているため、実際に自分でRubyのコードを動かさなくてもどのような挙動になるかがわかります。そのため、PCの前にいなくてもRubyの挙動を確認できます。「自分で手を動かさないと理解は進まない!」派の人もいるでしょうが、電車の中のようにPCの前にいないときでも動作を確認しながら読み進められると便利な人も多いでしょう。

なんとなくRubyを触っている人のステップアップ用だけではなく、日常的にRubyを使っている人が最新の情報を仕入れるための用途にも便利な内容です。Rubyのことはだいたい知っているという人でも読んでみてはいかがでしょうか。なにかしら知らなかった機能が見つかることでしょう。

この本の特徴は網羅性ですが、それがネックでもあります。たくさんの情報を盛り込んでいるため、ページ数は600ページ超になっています。読むことが大変なのです。

機能の紹介は淡々と続いていきます。どうしてこの順番で解説しているか、そのようなストーリーの説明はあまりありません。おそらく、解説したい情報がたくさんあったのでしょう。そして、それらをできるだけ盛り込もうとしたらつながりを説明する余白がなくなったのでしょう。

サンプルコードは豊富にありますが、図はそれほどありません。説明でもコードでもピンとくる場合は多くあるでしょうが、そうでないこともあるでしょう。その場合、図で視覚的にイメージできると理解の助けになりますが、そのようなチャンスはそれほどありません。

Rubyの機能をあまり知らない人であれば「このような機能もあるのか!」という新鮮さがあるため、機能をどんどん紹介するというこの本の構成で楽しく読み進めていけるでしょう。しかし、すでに日常的にRubyを使っている人であれば、知っている情報がたくさんあるのでこの構成では飽きてしまうかもしれません。日常的にRubyを使っている人は流し読みをしながら見慣れない機能のところをかいつまんで読むのがよいでしょう。

Rubyの楽しさはあまり伝わらないかもしれませんが、網羅性ではたしかに「パーフェクト」といえるでしょう。この1冊でRubyの多くの機能を知ることができます。Rubyに興味がある人は読んでみてはいかがでしょうか。

*1 知ったからといって日常的に使う必要があるわけではない。本当にこれらを使うのが適切なときに思い出して使えれば十分。

*2 この本は複数の著者が書いていますが、サンプルコード内のサンプルの値やホームディレクトリのパスなどで誰が書いているかが透けてみえるのが興味深いです。

2013-08-19

インターンシップで学んだこと3:テストを整理する方法

インターン募集を開始したのが半年前の2月で、6月に開催されたRubyKaigi 2013までは1件も応募がありませんでした。RubyKaigi 2013に参加したところ1件応募があり、6月後半から7月にかけて実施しました。ここ最近まとめているインターンシップで学んだことはこの時期に実施したインターンシップで学んだことです。

その後、さらに2件の応募がありました。クリアコードのインターンシップは学生に限定せず働いている人でも働いていない人でも対象としていますが、3件ともすべて学生の方です。学生の方からの応募は夏休みの時期の方が多いようです*1

2件の応募のうち、1件は「インターンシップ実施企業と学生が話をできる場を提供するイベント」がきっかけでした。もう1件はリーダブルコードの解説を読んでクリアコードを知ったことがきっかけだということでした。解説を書いてよかったです*2

なお、この2件の応募に対して9月にインターンシップを実施する予定です。9月のインターンシップに向けて、前回のインターンシップで学んだことを早くまとめて活かしたいところです。が、間に合わなそうです。17日あるうちのまだ3日目です。

さて、前回は3日目に3つ学んだことの中の1つ「1人で開発しているときにtypoとどうつきあっていくか」についてまとめました。今回は3日目に学んだことの2つめである「テストを整理する方法」についてまとめます。

冗長なテスト

3日目の時点でのテストは以下のように50行にも満たない小さなものでした。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
# -*- coding: utf-8 -*-

require 'test-unit'

require 'epub/parser'
require 'epub-searcher/epub-document'

class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors
    assert_equal([], @document_1.extract_contributors)
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end

  def test_extract_creators
    assert_equal(["groonga"], @document_1.extract_creators)
    assert_equal(["groongaプロジェクト"], @document_2.extract_creators)
  end

  def test_extract_title
    assert_equal("groongaについて", @document_1.extract_title)
    assert_equal("groongaについて", @document_2.extract_title)
  end

  def test_extract_xhtml_spine
    assert_equal(["OEBPS/item0001.xhtml"], @document_1.extract_xhtml_spine)
    assert_equal(["item0001.xhtml", "item0002.xhtml"], @document_2.extract_xhtml_spine)
  end

  private
  def fixture_path(basename)
    File.join(__dir__, 'fixtures', basename)
  end
end

しかし、いくつか冗長な点が見えてきています。例えば変数名です。

1
2
3
4
5
6
7
8
9
10
11
def setup
  # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
  # 本文は groonga ドキュメント 1 章 が全て入っている
  epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
  @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

  # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
  # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
  epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
  @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
end

単にepub_book@documentとするのではなく、epub_book_1@document_2としています。epub_book_@document_が重複しています。

同様に、テストメソッド名にも冗長な点があります。すべてのテストメソッド名がextract_を含んでいます。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
def test_extract_contributors
  # ...
end

def test_extract_creators
  # ...
end

def test_extract_title
  # ...
end

def test_extract_xhtml_spine
  # ...
end

このように、重複した情報によりテストが冗長になってきています。

流れ的には「冗長なテストをすっきりさせるにはどうしたらよいか」という話にいくのですが、その前に、どうして冗長なテストをすっきりさせなければいけないかを考えてみましょう。

冗長なテストをすっきりさせなければいけない理由

実装のコードはすっきりさせないといけないと言われています。そのための技術としてリファクタリングという名前もついているぐらいです。では、テストのコードはどうでしょうか。テストのコードもすっきりさせないといけないのでしょうか。もし、テストのコードもすっきりさせないといけないなら、テストのコードにもリファクタリングという技術を使えるのでしょうか。

結論を先にいうと、テストのコードもすっきりさせなければいけません。しかし、リファクタリングという技術は使えません。テストのコードをすっきりさせるには別のやり方を使わなければいけません。

テストのコードをすっきりさせなければいけない理由は、実装のコードをすっきりさせなければいけない理由と同じです。コードがすっきりしていないと、新しくコードを追加したり、既存のコードを修正したりといったコードを変更することが難しくなるからです。

では、テストのコードを変更しやすくするのはどうしてでしょうか。それは、実装のコードを変更しやすくするためです。テストがあれば、実装のコードを整理しても整理したことによって実装が壊れていないことを簡単に確認できます。同様に、新しく機能を追加したときも既存の機能が壊れていないことを簡単に確認できます。テストのコードが変更しづらくなって、テストを追加しなくなっていくと、実装のコードが壊れてしまったかどうかを確認することが難しくなります。そうすると、こわくて実装のコードを変更しづらくなります。

テストのコードが変更しやすいと以下のように開発できます。

実装変更 + テスト追加 → テスト実行 → 問題なし →
実装変更 + テスト追加 → テスト実行 → 問題なし →
実装整理              → テスト実行 → 問題なし →
実装変更 + テスト追加 → テスト実行 → 問題なし →
…

しかし、テストのコードの変更がしづらいと以下のようになります。

実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装整理                   → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) → テスト実行 → たぶん問題なし →
実装変更(テスト追加なし) →
テスト少ないし実行しなくてもいいよね →
問題ないといいなぁ →
実装変更(したくないなぁ) →
…

まとめると、テストのコードを変更しやすくしておくことは、実装を変更しやすくしておくことにつながるので重要です。

それでは、テストのコードをすっきりさせるにはどうしたらよいでしょうか。残念ながらリファクタリングという技術は使えません。なぜなら、リファクタリングをするにはテストが必要だからです。テストをリファクタリングするにはテストのテストが必要になります。これではいつまでたってもテストのリファクタリングができません。

テストをすっきりさせるにはどのようなやり方がよいかを考えてみましょう。

それぞれのテストを独立させる

テストのコードを整理するときは、変更が他のテストに影響を与えていないことを手動で確認する必要があります。テストのテストがないため自動で確認することができないからです。

手動で確認するので、多くのことを確認しなければいけない状況は避けましょう。ミスが多くなります。この状況を避けるためには、それぞれのテストの影響範囲を小さくしておくことが有効です。変更の影響を確認する範囲が小さくなります。

具体的には以下のようにしてそれぞれのテストを独立させます。

  • 1つのテストで1つのことを確認する。
  • テスト対象ごとにテストをグループ化する。(さまざまなテスト対象のテストを1つのグループにまとめない。)
1つのテストで1つのことを確認

1つのテストで1つのことを確認するというのは、ざっくりというと1つのテストの中で1つのアサーションを使うということです*3

今回のケースでは、以下のようにtest_extract_contributorsという1つのテストの中で、「spineを1つしか含まないケース」と「spineを2つ含むケース」の2つのことを確認していました。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors
    assert_equal([], @document_1.extract_contributors)
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

これは、以下のように別のテストにわけると1つのテストで1つのことを確認するようになります。

1
2
3
4
5
6
7
8
9
10
11
12
13
class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # ...
  end

  def test_extract_contributors_with_spine
    assert_equal([], @document_1.extract_contributors)
  end

  def test_extract_contributors_with_spines
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

こうすると、@document_1を変更しても2つめのテスト(..._spinesの方)には影響がありません。影響範囲が狭くなりましたね。

なお、1つのテストで1つのことを確認することはテストが失敗したときのデバッグのしやすさにもつながるので、テストの変更のしやすさ以外の観点からも有用です。

テスト対象ごとにテストをグループ化

個々のテストではなくもう少し大きな単位で考えてみましょう。影響範囲を小さくする別の方法があります。具体的に言うと、「テストをグループ化した単位」です。xUnitの場合はテストケースです。

具体的なコードで考えてみましょう。以下のコードには1つのテストケースの中に2つのテストがあります。それぞれのテストではsetupで用意したのに使っていない変数があります。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
class TestEPUBDocument < Test::Unit::TestCase
  def setup
    # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
    # 本文は groonga ドキュメント 1 章 が全て入っている
    epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
    @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)

    # groonga_doc_11_12.epub ... spine を二つ含む EPUB ファイル
    # 本文は groonga ドキュメント 1.1 と 1.2 が入っている
    epub_book_2 = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
    @document_2 = EPUBSearcher::EPUBDocument.new(epub_book_2)
  end

  def test_extract_contributors_with_spine
    assert_equal([], @document_1.extract_contributors)
  end

  def test_extract_contributors_with_spines
    assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document_2.extract_contributors)
  end
end

@document_1は1つめのテストでしか使われていません。@document_2は2つめのテストでしか使われていません。setupで準備したのに使われていない変数があるということは、そのテストケースでは様々な種類のテストをグループ化しているということです。必要な変数を使うテスト毎にグループを細かくすることで影響範囲を小さくすることができます。

「spineの数」でさらにグループ化しましょう。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
class TestEPUBDocument < Test::Unit::TestCase
  class TestSingleSpine < self
    def setup
      epub_book = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
      @document = EPUBSearcher::EPUBDocument.new(epub_book)
    end

    def test_extract_contributors
      assert_equal([], @document.extract_contributors)
    end
  end

  class TestMultipleSpines < self
    def setup
      epub_book = EPUB::Parser.parse(fixture_path('groonga_doc_11_12.epub'))
      @document = EPUBSearcher::EPUBDocument.new(epub_book)
    end

    def test_extract_contributors
      assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"], @document.extract_contributors)
    end
  end
end

「spineの数」でグループ化したことにより、テストケース名(TestSingleSpine)で「どのようなspineに注目しているか」ということが表現できるようになりました。そのため、もともとは以下のように「spineの数は1つだよ」と書いていたコメントを削除しました。

1
2
3
4
5
6
def setup
   # groonga_doc_all.epub ... spine を一つしか含まない EPUB ファイル
   #                          本文は groonga ドキュメント 1 章 が全て入っている
   epub_book_1 = EPUB::Parser.parse(fixture_path('groonga_doc_all.epub'))
   @document_1 = EPUBSearcher::EPUBDocument.new(epub_book_1)
end

使っている変数に注目してテストケースをわけたため、それぞれのテストは独立するようになりました。TestSingleSpineの中を変更しても、もう一方のテストケースには影響を与えません。

今は変数に注目してテストケースをわけましたが、メソッド名に注目してわける方法もあります。メソッド名の一部のtest_extract_contributorsに注目するとこうなります。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
class TestEPUBDocument < Test::Unit::TestCase
  class TestExtractContributors < self
    def test_empty
      assert_equal([], extract("groonga_doc_empty.epub"))
    end

    def test_multiple
      assert_equal(["groongaコミュニティ A", "groongaコミュニティ B", "groongaコミュニティ C"],
                   extract("groonga_doc_multiple_contributors.epub"))
    end

    private
    def extract(path)
      epub_book = EPUB::Parser.parse(fixture_path(path))
      document = EPUBSearcher::EPUBDocument.new(epub_book)
      document.extract_contributors
    end
  end

  def test_XXX
    # ...
  end

  # ...
end

extract_contributorsという情報をテストケース名にもっていって、テスト名からは抜きました。これで、このテストケース内の変更は他のテストケースには影響を与えません。

extract_contributorsがsnake_caseからExtractContributorsというCamelCaseになって気持ち悪いと感じる人もいるでしょう。その場合はsub_test_caseを使えば、snake_caseのまま書けます*4

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
class TestEPUBDocument < Test::Unit::TestCase
  sub_test_case("extract_contributors") do
    def test_empty
      # ...
    end

    def test_multiple
      # ...
    end

    private
    def extract(path)
      # ...
    end
  end

  def test_XXX
    # ...
  end

  # ...
end

テストのグループ化についてはテストをすっきり書く方法として過去にまとめていて、インターンシップでもこのエントリーを読んでもらいました。

まとめ

インターンシップで説明した、テストを整理する方法についてまとめました。今回説明したやり方は以下の2つです。

  • 1つのテストで1つのことを確認する。
  • テスト対象ごとにテストをグループ化する。

過去にこれらのやり方についてエントリーにまとめていたので、インターンシップではまずエントリーを読んでもらい、概要を把握してもらったあとに説明しました。知見を文章としてまとめておくと有用であることがわかりました。過去に説明したものはまとめておいて、次に説明する機会があるときに参照しながら説明する、ということは今後も続けたいです。

作業をしていてなにかあったら相談して欲しいということは伝えていましたが、それだと曖昧すぎるようです。それよりも、「名前に困ったら相談するタイミング」というより具体的な基準を設定するほうが実行しやすいということがわかりました。今回のケースでは_1_2がでていますが、連番は適切な名前が思いつかなかったときに使われることが多いです*5。このように、この基準はわかりやすいため、これからも使えそうです。

*1 そりゃそうだろうなぁという感じですね。

*2 リーダブルなコードに関する講演依頼はお問い合わせフォームからどうぞ。

*3 複数のアサーションで1つのことを確認しているならそれでもOKです。厳密に1つのアサーションにしろということではなく、1つのことを確認しているかがわかりやすい表現なだけです。

*4 ただし、いつものRubyの書き方ではなくなります。この書き方を知っている人にはわかりやすいかもしれませんが、Rubyだけを知っている人には馴染みにくいでしょう。どちらを選ぶかはトレードオフを判断して決めてください。

*5 連番が適切なときもありますが、配列などで表現することもできるのでそんなに多くはありません。

2013-08-27

«前月 最新記事 翌月»
タグ:
年・日ごとに見る
2008|05|06|07|08|09|10|11|12|
2009|01|02|03|04|05|06|07|08|09|10|11|12|
2010|01|02|03|04|05|06|07|08|09|10|11|12|
2011|01|02|03|04|05|06|07|08|09|10|11|12|
2012|01|02|03|04|05|06|07|08|09|10|11|12|
2013|01|02|03|04|05|06|07|08|09|10|11|12|
2014|01|02|03|04|05|06|07|08|09|10|11|12|
2015|01|02|03|04|05|06|07|08|09|10|11|12|
2016|01|02|03|04|05|06|07|08|09|10|11|12|
2017|01|02|03|04|05|06|07|08|09|10|11|12|
2018|01|02|03|04|05|06|07|08|09|10|11|12|
2019|01|02|03|04|05|06|07|08|09|10|11|12|
2020|01|02|03|04|05|06|07|08|09|10|11|12|