#TDDBC 岡山参加者のプログラムを見て

TDDBC岡山にてJavaのTAをやっていて沢山の方々のプログラムを見せていただきました。
その中でいくつか気になったところがあるので書きたいと思います。

TDDBC岡山で課題として行われたのは自動販売機についてです。
以下のURLに課題がまとまっていますのでご参照下さい。
この記事はステップ1までの内容です。
http://devtesting.jp/tddbc/?TDDBC%E5%A4%A7%E9%98%AA2.0%2F%E8%AA%B2%E9%A1%8C

あくまで個人の見解でありこの内容が絶対正しいという気はありません。
また、基本JavaのTAでコードレビューのみC++も見させていただきました。
ですので、それらを見ての感想となります。

全体的な感想

さて、課題を先にすすめることを優先してリファクタリングがおろそかになっているように見受けられました。
例えばC++でTDDを行なっていたペアはすべてのコードをヘッダーファイルに記載していましたが、それを修正することはありませんでした。
きちんとリファクタリングを徹底させるべきではなかったかと反省しています。

個別の感想

個別の事案について書いていきたいと思います。

投入できるお金についての設計

最初に投入できるお金についての設計が行われていないペアが目立ちました。
ほとんどのペアのコードは以下のようになっていました。

public VendingMachine {
  private int totalAmount = 0;
  public void insert(int money) {
    totalAmount += money;
  }
}

この状態のコードだと私は以下の2点について検討したいです。

金額として負数はあり得るのか
50円玉、100円玉、500円玉、1000円札を１つずつ投入できる。とうたわれているのでそれ専用のクラスを作ったほうが良いのではないか*1

まず最初の案を検討した場合は私なら以下のようにするでしょうか。

public VendingMachine {
  private int totalAmount = 0;
  public void insert(int money) {
    if(money <= 0) {
      throw new IllegalArgmentException("money is negative.");
    }
    totalAmount += i;
  }
}

次の案を検討した場合は以下のようにします。

public VendingMachine {
  private int totalAmount = 0;
  void insert(Money money) {
    if(money == null) {
      throw new IllegalArgmentException("money is null");
    }
    totalAmount += money.getValue();
  }
}
public enum Money {
  FIFTY(50),
  ONE_HUNDRED(100),
  FIVE_HUNDREDS(500),
  ONE_THOUSAND(1000),
  private int value;
  Money(int value) {
    this.value = value;
  }
  public int getValue() {
    return value;
  }
}

ドキュメントの記載

JavaDocを記載しているチームはありませんでした。
TDDであってもJavaDocに関してはリファクタリング時にはきちんと書くべきだと思っています。

/**
 * 自動販売機に投入できるお金。
 */
public enum Money {

このクラスは"ステップ1"では以下の様なJavaDocに修正されます。

/**
 * 日本で流通しているお金。
 */
public enum Money {

ステップ0では投入できるお金のみを扱っていたのがステップ1で投入できないお金についての要件が出てきたからですね。
同じ名前でも違うものを表す場合があるのできちんとしたドキュメントは必要です。*2

テストケースの記述方法

テストケースをテストメソッド名として日本語で記述するスタイルで気になることがありました。
以下の様にテストケースを記載している人が多かったです。

@Test
public void 百円投入した場合() {
〜〜〜
}

これだと、何を期待しているのかが記載できていないので以下のように書いたほうが良いでしょう。

@Test
public void 百円投入した場合合計金額が百円となっていること() {
〜〜〜
}

実際、以下のようになりかけているペアもありました。

@Test
public void 百円投入した場合1() {
〜〜〜
}
@Test
public void 百円投入した場合2() {
〜〜〜
}

TDDがなぜ良いかという回答の一つに実際に作る前にin/outの設計をすることを義務付けている事があると思っています。
inだけ定義してoutについて定義しないというのは片手落ちになるのでご注意下さい。

Java特有の制限事項

お釣りを取得するという機能があります。
以下のように実装されているものがありました。

public VendingMachine {
  private int totalAmount = 0;
  〜〜〜
  public int getChange() {
    int charge = totalAmount;
    totalAmount =0;
    return charge;
  }
}

Javaではgetもしくはsetから始まるメソッドはアクセサとして特殊な意味を持っています。
副作用がある操作は定義しないほうが良いです。
そうしないと困る人が出てくることもあります。

以下のようにしたほうが良いでしょう。

public VendingMachine {
  private int totalAmount = 0;
  〜〜〜
  public int pickChange() {
    int charge = totalAmount;
    totalAmount = 0;
    return charge;
  }
}

まとめ

これらが私がTAをしていて気になったところです。
TDDを行なっていて実装が上手く進んでいると細かい設計や命名規則が乱雑になりやすいのかなとも思いました。
改めて言いますが私が気になったところというだけでこれが正しいというわけではないです。
ただ、同意していただけるのであれば気をつけてくださると嬉しいです。
それでは良いTDDライフを！

(追記)
以下の本をLTで紹介させていただきました。

ThoughtWorksアンソロジー ―アジャイルとオブジェクト指向によるソフトウェアイノベーション

作者: ThoughtWorks Inc.,株式会社オージス総研オブジェクトの広場編集部
出版社/メーカー: オライリージャパン
発売日: 2008/12/27
メディア: 単行本（ソフトカバー）
購入: 14人クリック: 323回
この商品を含むブログ (81件) を見る

Amazonでは売り切れていますがオライリーでは買えますのでぜひ！
http://www.oreilly.co.jp/books/9784873113890/

*1:そもそも現実世界で扱えるお金は硬貨、もしくは紙幣に限られます。

*2:クラス名を自動販売機に投入できるお金相当の英語にするのはさすがにやりすぎだと思います。