[ Java Project Exampleページへ戻る ]

試験

時刻プログラムの試験などの活動。

コンテンツ

ユニットテスト

イテレーション No.2

ユニットテストを導入した。Clockクラスに対応するテストケースClockTest.javaを作成した。JUnitで作成しており、テスト項目についてはAPIドキュメントを参照のこと。

コードレビュー

イテレーション No.1

レビュー指摘事項

  1. アクセッサ以外の場所でフィールドを直接変更している（コンストラクタの中）
    コンストラクタ中でのフィールドを直接参照して値を設定している。フィールドはアクセッサを必ず使用して読み書きする規約に反している。なお、この問題に対処する際の注意点を挙げておく。コンストラクタ中でアクセッサを使用する場合、アクセッサメソッドをfinal宣言しておかないと、アクセッサがサブクラスでオーバーライドされてしまったときに意図しない挙動が生じる。
  2. 即値を使用している（コンストラクタ、およびSetterメソッドの中）
    引数に指定された値が事前条件を満たしているか判定するロジックにおいて、複数箇所で値の範囲を即値で記述している。値が変更になった場合修正漏れが生じる危険性がある。
  3. 同じコードが繰り返し現れている（コンストラクタ、およびSetterメソッドの中）
    引数に指定された値が事前条件を満たしているか判定するロジックが、繰り返し登場している。このような場合、繰り返し表れる部分をメソッドとして独立されることが一般的である。

レビュー指摘事項の対処

  1. 規約違反だが、今回のイテレーションではそのままとする。設計上コンストラクタの使用は暫定的なものであり、今後のイテレーションの中で変わることが想定されるためである。
  2. 次のイテレーションで対処する。
  3. 次のイテレーションで対処する。

イテレーション No.2

レビュー指摘事項

1.、2.、および3.は、イテレーションNo.1におけるレビュー指摘事項を詳細化した記述である。

  1. アクセッサ以外の場所でフィールドを直接変更している
    1. Clock(int, int, int, int)の中で、hour/minute/second/timerの各フィールドを直接更新している
    2. start()の中で、isStartフィールドを直接参照および更新している
    3. stop()の中で、isStartフィールドを直接参照および更新している
    4. startUpdate()の中で、taskフィールドを直接更新している
    5. stopUpdate()の中で、taskフィールドを直接更新している
    6. インナークラスClockUpdateTaskのrun()の中で、hour/minute/secondの各フィールドを直接更新している
  2. 即値を使用している
    1. Clock()の中で、時分秒の指定に即値 0, 0, 0 を使用している
    2. Clock(int, int, int, int)の中で、時の有効範囲判定に即値 0, 23を使用している
    3. Clock(int, int, int, int)の中で、分の有効範囲判定に即値 0, 59を使用している
    4. Clock(int, int, int, int)の中で、秒の有効範囲判定に即値 0, 59を使用している
    5. Clock(int, int, int, int)の中で、インターバルの有効範囲判定に即値 0を使用している
    6. setHour(int)の中で、時の有効範囲判定に即値 0, 23を使用している
    7. setMinute(int)の中で、分の有効範囲判定に即値 0, 59を使用している
    8. setSecond(int)の中で、秒の有効範囲判定に即値 0, 59を使用している
    9. setInterval(int)の中で、インターバルの有効範囲判定に即値 1を使用している
    10. インナークラスClockUpdateTaskのrun()の中で、時分秒のオーバーフロー判定に即値 60, 0, 24を使用している
  3. 同じコードが繰り返し現れている
    1. Clock(int, int, int, int)の中と、setHour(int)の中で、時の有効範囲判定を行うコードが繰り返し現れている
    2. Clock(int, int, int, int)の中と、setMinute(int)の中で、分の有効範囲判定を行うコードが繰り返し現れている
    3. Clock(int, int, int, int)の中と、setSecond(int)の中で、秒の有効範囲判定を行うコードが繰り返し現れている
    4. Clock(int, int, int, int)の中と、setInterval(int)の中で、インターバルの有効範囲判定を行うコードが繰り返し現れている
      （判定が、<=0 と <1 と表現が異なっているが同じ判定となっている）
  4. マルチスレッド対応が行われていない
    Timerを導入したことにより、マルチスレッド・プログラミングの必要性が生じる。Timerにスケジュールされた処理と、それ以外の処理は、別なスレッドで実行されるからだ。
    1. TimerのスレッドにおいてインナークラスClockUpdateTaskのrun()の中で時分秒の更新を行うが、別なスレッドからgetHour()/getMinute()/getSecond()が呼ばれたとき、整合が取れた時分秒が返されない可能性がある。
  5. ローカル変数がフィールドの名前を隠蔽している
    1. stratUpdate()の中で、ローカル変数intervalは、フィールドintervalの名前隠蔽となっている
  6. ClockクラスはtoStringメソッドをオーバーライドしていない
    オブジェクトの状態を文字列化するtoStringをオーバーライドすべきである。
    1. ObjectクラスのtoStringメソッドが呼ばれるため、オブジェクトのハッシュ値が文字列化される。Clockオブジェクトにとってハッシュ値は状態を表現するに不十分である。

レビュー指摘事項の対処方法

  1. すべてアクセッサを利用するようにコードを修正する。
    1. hour/minute/secondのフィールドについては、アクセッサsetHour/setMinute/setSecondを介して更新する。timerについては、timer初期化メソッドinitTimer()を新規に設ける。
    2. isStartフィールドの参照は、isStart()を介する。isStartフィールドの更新は、SetterメソッドsetStart(boolean)を新規に設ける。
    3. 同上
    4. taskフィールドは、startUpdate()/stopUpdate()からだけ参照される。また、値が変わるのではなく、オブジェクトを参照するかnullかのどちらかである。ここにアクセッサ（Setter/Getter)を持ち込むのはどうかと思い、対処はしないこととする。
    5. 同上
    6. アクセッサを介して更新する（4.1と重なるので、4.1参照）
  2. すべて定数を定義してそれを利用するようにコードを修正する。
    1. 定数DEFAULT_HOUR/DEFAULT_MINUTE/DEFAULT_SECONDを定義してこれを利用する
    2. 定数MIN_HOUR/MAX_HOURを定義してこれを利用する(3.1と重なるので3.1参照）
    3. 定数MIN_MINUTE/MAX_MINUTEを定義してこれを利用する（3.2と重なるので3.2参照）
    4. 定数MIN_SECOND/MAX_SECONDを定義してこれを利用する（3.3と重なるので3.3参照）
    5. 定数MIN_INTERVALを定義してこれを利用する（3.4と重なるので3.4参照）
    6. 2.と同じ
    7. 3.と同じ
    8. 4.と同じ
    9. 5.と同じ
    10. 定数MAX_HOUR/MAX_MINUTE/MAX_SECONDを利用する
  3. 範囲チェック用メソッドを定義してこれを利用する
    1. checkHourCondition()を定義して利用する
    2. checkMinuteCondition()を定義して利用する
    3. checkSecondCondition()を定義してこれを利用する
    4. checkIntervalCondition()を定義してこれを利用する
  4. スレッドセーフな設計を行う
    1. インナークラスClockUpdateTaskのrun()の中でhour/minute/secondを更新している過渡状態において、他のスレッドからgetHour()/getMinute/getSecond()が呼ばれたときは、過渡状態が終了するまでブロックする。方法は、hour/minute/secondを一括して変更する同期メソッドsetTime(int, int, int)を追加する。
  5. ローカル変数名を変更する
    1. フィールドは必ずアクセッサを介するので、ローカル変数と名前がかぶったとしてもあまり気にしなくてもよいとは思う。が、コーディング標準に従って改名する。currentInterval とでもしておこう。
  6. toStringメソッドをオーバーライドする
This page is written by Toru TAKAHASHI.(torutk@02.246.ne.jp)