DecimalAmount型の機能拡張（桁数の異なる計算）
Closed this issue · 16 comments
Pull Request #21 で初期バージョンの完了。
最大・最小は、厳密には、Long.MAX Long.MIN ではなく、 Long.MAX / 桁数分、 Long.MIN / 桁数分になるはず。
BigDecimal#unscaledValue() が、Long.MAX, Long.MIN に収まるように検査すればよさそう。
以下ようなイメージでしょうか？
Long.MAX の Amount 型から、 小数点以下１桁の DecimalAmount 型は生成できない
はい。そういう認識です。
最大金額は、Amount型の最大金額 (LONG_MAX)の 100分の１である
すみません。もとの Issue #21 に記載されていましたね。
内部を BigDecimal にしたので、整数部分を Long.MAX, Long.MIN まで可能にしてしまいました。
異なる桁数の加算減算については、 domain.type パッケージなので可能にして良いと思いました。
（123.40 と 123.4 や 123.00 と 123 は、同じように加算、減算できて良い）
桁の制限が必要な場合は、 domain.model 側に任せる形です。
（例えば、少数点以下2桁に固定のみ DecimalAmount 扱うなど）
下記のような実装はいかがでしょうか？
- BigDecimal の add(BigDecimal) の用に、桁が大きい方に合わせる
- 加算、減算で 桁数が変わって、Long.MAX / 桁数分、Long.MIN / 桁数分 の範囲外となった場合は例外とする
- BigDecimal の add(BigDecimal) の用に、桁が大きい方に合わせる
情報が（不用意に）失われないようにするわけですね。
いったんは、この仕様でよいと思います。
将来的には、 Math#addExact() のように桁数の違いを厳密にチェックするメソッドを追加するか、ExactDecimalAmountなど別クラスで、桁数の違いを厳密にチェックするクラスを用意するかというあたりを検討したいとは思っています。（別 issue #48 を追加）
- 加算、減算で 桁数が変わって、Long.MAX / 桁数分、Long.MIN / 桁数分 の範囲外となった場合は例外とする
はい。これが安全な仕様だと思います。
桁の異なる計算を実装してみました。
（BigDecimal で計算していたため、加算、減算は修正していません）
桁数が可変になったため、以下の修正をしています。
valueOf(Amount) は 整数 で生成
valueOf(String) は 小数点以下４桁まで生成可能
最大、最小の範囲を Long.MAX / 桁数分、Long.MIN / 桁数分 に修正
必要な仕様が、シンプルに実装できていますね。良い感じです。
コードを読んでいて、インスタンス変数名の value と、メソッドの引数名の value が、同じなのが気になりました。
頭の中で衝突することがあって、ちょっと読みにくく感じた。
インスタンス変数は、他のクラスでも、value としているので、同じにしておきたいかな。
引数のほうを変えるとしたら、Java の慣習的な引数名って、なんでしょうかね？
思いつくところをあげると、
- other
- source
- 型名の小文字 ( Amount amount とか)
- valueToAdd のように to とか for で修飾する
- longValue のように、value の前に、プリミティブな型名で修飾する
他にもいろいろありそうですね。
少し、検討してみていただけませんか？
一つに決めるとか、ルール化はしたくないですね。
どんな引数名だと、そのメソッドの説明として、より、しっくりくるか（読みやすいか）ですね。
確認ありがとうございます。
コードを読んでいて、インスタンス変数名の value と、メソッドの引数名の value が、同じなのが気になりました。
たしかに、わかりにくいですね。。
以下のように使い分けてみました。
- オブジェクトを作成する類は、longValue, amount, source (String, BigDecimal)
- 比較メソッドは other
- 演算はそのまま(value以外を使用していたので)
ありがとうございます。
読みやすくなった感じがします。
#53 で議論したコメントでの補足説明を、このクラスで実験してみましょう。
BigDecimalほど、詳細でなくても良いですが、桁数についてｍこのissueで議論してきた内容の要点をクラスレベルのJavaDocコメントとして書いてみていただけますか？
BigDecimal を参考に小数点以下の説明を記載してみました。
scale() メソッドがないので、違和感があります。。
ありがとうございます。
桁数は、やはり、このような説明をドキュメントとして書いておくべきですね。
コードからだけでは、意図が読み取りにくい。
この説明によって、意図がはっきりします。
読んでみて、以下の点を変更すると、もっとわかりやすくなると思いました。
- DecimalAmount クラスは {@link java.lang.Long} の整数値と 0から4までの小数点以下を表すスケールで構成されます。
... 整数値 (unscaled value) と0から4までの小数点以下の桁数(scale)で構成します。
表される → 表わす （可能であれば、受動態より能動態のほうがわかりやすい）
までで、 → の範囲です。 (文として区切ったほうがよさそう）
- 加算、減算は、異なるスケールの演算が可能ですが、乗算、除算は乗数、除数は整数値のみとなります。
- 加算と減算は、異なるスケールの演算が可能です。
- 乗算の乗数は整数値のみです。
- 除算の除数は整数値のみです。
（一文で書くより、分解してしまったほうが読みやすそう）
テーブルは、カラムをもう一列追加して「説明」として、日本語の説明も追加したほうが、より伝わりやすいかな、と思いました。
少し、冗長かもしれませんが、今後、仕様の追加や変更をしていく可能性が高いので、基本の枠組みと書き方の指針として、上記のような変更がよいかと思いました。
確認ありがとうございます。
修正しました。
わかりやすくなったと思います。
ありがとうございます。
良い感じですね。
確認ありがとうございます。
この内容でプルリク出しますね。