Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

第13章 実装を導くテスト #17

Merged
merged 20 commits into from
Nov 10, 2017
Merged

第13章 実装を導くテスト #17

merged 20 commits into from
Nov 10, 2017

Conversation

at-grandpa
Copy link
Owner

@at-grandpa at-grandpa commented Nov 9, 2017

TODO

  • $5+10CHF=$10(レートが2:1の場合)
  • $5+$5=$10

@at-grandpa
Copy link
Owner Author

仮実装のreduceメソッドのMoney.dollar(10)が重複している。

@at-grandpa
Copy link
Owner Author

細かく行くところだが、今回は進んで見る。TODOにひとつ追加。

@at-grandpa
Copy link
Owner Author

at-grandpa commented Nov 9, 2017

TODO

  • $5+10CHF=$10(レートが2:1の場合)
  • $5+$5=$10
  • $5+$5がMoneyを返す

@at-grandpa
Copy link
Owner Author

ここで、ExpressionクラスはSumクラスなどの親っぽいクラスであることに注意。

@at-grandpa
Copy link
Owner Author

plusメソッドはExpressionのクラス、もといSumクラスを返す必要がある。

@at-grandpa
Copy link
Owner Author

これをテストに加える。

@at-grandpa
Copy link
Owner Author

当然、Sumがないのでテストは落ちる。

crystal spec' <

Error in line 1: while requiring "./spec/money_package_spec.cr"

in spec/money_package_spec.cr:39: undefined constant Sum

      sum : Sum = result.as(Sum)
            ^~~

@at-grandpa
Copy link
Owner Author

このテストは長生きするとは思っていない。内部実装に深く関連したテストだからだ。外部から見た振る舞いのテストにはなっていない。

@at-grandpa
Copy link
Owner Author

実装はしたが、crystal特有のコンパイルエラーになる。

 crystal spec' <

Error in src/money_package/sum.cr:3: expanding macro

    property augend : Money
    ^

in macro 'property' expanded macro: macro_4490484464:567, line 4:

   1.
   2.
   3.
>  4.             @augend : Money
   5.
   6.             def augend : Money
   7.               @augend
   8.             end
   9.
  10.             def augend=(@augend : Money)
  11.             end
  12.
  13.
  14.
  15.

instance variable '@augend' of MoneyPackage::Sum was not initialized in all of the 'initialize' methods, rendering it nilable

augendaddendの初期値が設定されていない。

@at-grandpa
Copy link
Owner Author

Nilを許容するようにして、この部分のコンパイルを通した。

@at-grandpa
Copy link
Owner Author

at-grandpa commented Nov 9, 2017

現在のコンパイルエラーはこれ。

 crystal spec' <

Error in spec/money_package_spec.cr:39: can't cast MoneyPackage::Money to MoneyPackage::Sum

      sum : Sum = result.as(Sum)
                  ^

javaのClassCastExceptionと同じようなことだろう。plusの返す型をSumにする。

@at-grandpa
Copy link
Owner Author

コンパイルエラー。

 crystal spec' <

Error in line 1: while requiring "./spec/money_package_spec.cr"

in spec/money_package_spec.cr:29: instantiating 'MoneyPackage::Money#plus(MoneyPackage::Money)'

      sum : Expression = five.plus(five)
                              ^~~~

in src/money_package/money.cr:23: wrong number of arguments for 'MoneyPackage::Sum.new' (given 2, expected 0)
Overloads are:
 - Reference.new()

      Sum.new(self, addend)
          ^~~

Sumにinitializeがない。

@at-grandpa
Copy link
Owner Author

コンパイルエラー。

crystal spec' <

Error in line 1: while requiring "./spec/money_package_spec.cr"

in spec/money_package_spec.cr:29: instantiating 'MoneyPackage::Money#plus(MoneyPackage::Money)'

      sum : Expression = five.plus(five)
                              ^~~~

in src/money_package/money.cr:22: type must be MoneyPackage::Expression, not MoneyPackage::Sum

    def plus(addend : self) : Expression
        ^~~~

plusが返す型はExpressionじゃないとだめだ。

@at-grandpa
Copy link
Owner Author

コンパイルエラーは通ったが、テストが落ちる。

 crystal spec' <

....F

Failures:

  1) MoneyPackage testPlusReturnsSum() pulsメソッドはSumクラスを返すこと
     Failure/Error: sum.augend.should eq five

       Expected: #<MoneyPackage::Money:0x10f6f7b40 @amount=5, @currency="USD">
            got: nil

     # spec/money_package_spec.cr:40

Finished in 1.17 milliseconds
5 examples, 1 failures, 0 errors, 0 pending

Failed examples:

crystal spec spec/money_package_spec.cr:36 # MoneyPackage testPlusReturnsSum() pulsメソッドはSumクラスを返すこと

Sumのinitializeでプロパティに値を入れていないから。

@at-grandpa
Copy link
Owner Author

テストが通った。

 crystal spec' <

.....

Finished in 942 microseconds
5 examples, 0 failures, 0 errors, 0 pending

@at-grandpa
Copy link
Owner Author

  • BankのreduceはSumを引数に取る
  • Sumで足される通貨が同じなら足し算の結果の通貨も同じになる
  • reduceで返されるのは足し算の結果を持ったMoney

@at-grandpa
Copy link
Owner Author

テストは落ちる。

 crystal spec' <

.....F

Failures:

  1) MoneyPackage testReduceSum() reduceメソッドのテスト
     Failure/Error: result.should eq Money.dollar(7)

       Expected: #<MoneyPackage::Money:0x100930a80 @amount=7, @currency="USD">
            got: #<MoneyPackage::Money:0x100930aa0 @amount=10, @currency="USD">

     # spec/money_package_spec.cr:49

Finished in 890 microseconds
6 examples, 1 failures, 0 errors, 0 pending

Failed examples:

crystal spec spec/money_package_spec.cr:45 # MoneyPackage testReduceSum() reduceメソッドのテスト

@at-grandpa
Copy link
Owner Author

コンパイルエラー

 crystal spec' <

Error in line 1: while requiring "./spec/money_package_spec.cr"

in spec/money_package_spec.cr:31: instantiating 'MoneyPackage::Bank#reduce(MoneyPackage::Sum, String)'

      reduced = bank.reduce(sum, "USD")
                     ^~~~~~

in src/money_package/bank.cr:5: undefined method 'amount' for Nil (compile-time type is (MoneyPackage::Money | Nil))

      amount = sum.augend.amount + sum.addend.amount
                          ^~~~~~

Rerun with --error-trace to show a complete error trace.

augendが (MoneyPackage::Money | Nil)という型なので、Nilの可能性がある。

@at-grandpa
Copy link
Owner Author

コンパイルエラー

 crystal spec' <

Error in line 1: while requiring "./spec/money_package_spec.cr"

in spec/money_package_spec.cr:31: instantiating 'MoneyPackage::Bank#reduce(MoneyPackage::Sum, String)'

      reduced = bank.reduce(sum, "USD")
                     ^~~~~~

in src/money_package/bank.cr:5: undefined method 'amount' for MoneyPackage::Money

      amount = sum.augend.amount + sum.addend.amount
                          ^~~~~~

Rerun with --error-trace to show a complete error trace.

amountはprivateだった。

@at-grandpa
Copy link
Owner Author

テスト通った。

 crystal spec' <

......

Finished in 657 microseconds
6 examples, 0 failures, 0 errors, 0 pending

@at-grandpa
Copy link
Owner Author

悪臭を放っている。

  • キャストをを行っている。本当はどんなExpressionでも動作しないといけない。
  • Sumのフィールドにアクセスしている。そのフィールドのフィールドにアクセスしている。

@at-grandpa
Copy link
Owner Author

なるほど。チェインの連鎖はこうやって移動して短くするのか。

@at-grandpa
Copy link
Owner Author

at-grandpa commented Nov 9, 2017

TODO

  • $5+10CHF=$10(レートが2:1の場合)
  • $5+$5=$10
  • $5+$5がMoneyを返す
  • Bank.reduce(Money)

@at-grandpa
Copy link
Owner Author

at-grandpa commented Nov 10, 2017

Bankのreduceメソッドに渡すExpressionがMoneyインスタンスの場合はどうするか、をTODOリストに加えた。

今は足し算のオブジェクトを渡しているが、Moneyを渡す時ももちろんあるので、このケースを思い出したからTODOに入れた。

@at-grandpa
Copy link
Owner Author

現在はグリーンバーなので、TODOリストのテストを書こう。

「Bank.reduce(Money)」だ。

@at-grandpa
Copy link
Owner Author

グリーンにしたのでリファクタリングしていこう。

@at-grandpa
Copy link
Owner Author

クラスの明示的なチェックはポリモーフィズムに置き換えるべき。

今はMoneyとSumでインターフェースが異なるので、このような分岐が発生している。Moneyもreduceを実装すれば、reduceの宣言をExpressionに移動できる。

@at-grandpa
Copy link
Owner Author

ExpressionとBankが同じreduceを持っているし、シグネチャが違う。キーワード引数とかだと、うまく表現できるかもしれないが、とりあえず今はこのようにする。

@at-grandpa
Copy link
Owner Author

at-grandpa commented Nov 10, 2017

TODO

  • $5+10CHF=$10(レートが2:1の場合)
  • $5+$5=$10
  • $5+$5がMoneyを返す
  • Bank.reduce(Money)
  • Moneyを変換して換算を行う
  • Reduce(Bank, String)

@at-grandpa
Copy link
Owner Author

振り返り。

  • 重複を除去できていないので、TODOリストの項目を「済」にしなかった
  • 実装の着想を得るためにさらに先に進むことにした
  • 後に必要になりそうなオブジェクト(Sum)の作成を促すテストを書いた
  • 速やかに実装を行った(Sumのコンストラクタ)
  • キャストを使って以下k書で実装した後で、テストが通るままで本来あるべき場所にコードを移した
  • ポリモーフィズムを使って明示的なクラスチェックを置き換えた

@at-grandpa
Copy link
Owner Author

13章終了。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants