The Wise Developers’ Guide to Static Code Analysis featuring FindBugs, Checkstyle, PMD, Coverity and SonarQube

zeroturnaround.com
ちょっと古い記事だけどよくまとまっているのと彼ら自身の経験談でもあるのでメモしておくw

ZeroTurnaround uses SonarQube for our Java projects JRebel and LiveRebel, as we find that it gives us the best combination of usefulness and transparency against the relative complexity of setting it up and solving weird configuration issues.

kotlin における例外について

ふと思ったのでメモ。

  • 例外なくアーキテクチャレベルでの扱いとなり、アーキテクチャ設計書で解説されるべきとか。
  • java で言うところの検査例外は sealed class の戻り値で扱うとか。
  • 検査例外のバケツリレーが発生するような場所はアーキテクチャレベルで実行時例外*1としての扱いを検討するのが良さげとか。*2

preconditions, postconditions and assertions - Be an Idealistic Realist も参照のこと。

*1:kotlin上ならRuntimeExceptionである必要は無いけどjavaとの互換性を考えると実行時例外にしておいたほうが無難だと思われ。

*2:多くの場合、実行時例外の発生可能範囲、ハンドリングを行う境界、具体的なクラスや仕組みなどが定義されると思われ。例えば、エラーが起きてもエラーメッセージだけ表示させてアプリを落とさないようなユーザー要件がある場合に、実行時エラーをスローしてUI側で受けてエラー表示と握りつぶしをする仕組みとか。

kotlin における検査例外的なものについて

ふと思ったのでメモ。

おいらは java では検査例外を結構気に入って使っていたが、kotlin では実行時例外しかないので微妙に困った。しかし、そもそも検査例外に満足していたわけでもない。

基本的にはインタフェースメソッドの仕様はユースケースに近い考え方で、主経路の結果が戻り値、代替経路の結果が例外というような考え方をしている。しかしこれは、様々な側面を考慮したうえで java 言語仕様に合わせた結果こうなったというだけの話で、理想形と言うわけではない。という感じで、いくつか思うところがある:

  • 例外を代替経路の結果としてとらえた場合、常に例外の型だけで代替経路を識別できるわけではない。複数の代替経路で同じ型の値を返したい場合には、代替経路の種類と値の両方の識別が必要となる。kotlin の場合、例外を使わず sealed class で対応し、sealed class の型を経路識別に利用し、経路の網羅性をコンパイラにチェックされるというのが落としどころですかね。
  • 複数の戻り値を得たほうが便利なこともある。純粋関数以外を認めないというのであればそれもいいが、関数は処理の段階的詳細化でも利用される。その際、戻り値が1つだとめんどくさいこともある。例えば x, y, z という int 値を受け取りたい場合、ポインタ渡しのほうがラクチンというようなケースもある。実際はポインタはバグを生みやすいので、kotlin なら destructuring でそれっぽい感じにするというのが落としどころな気はする。val (x, y, z) = hoge() 的な感じで。*1
  • 各経路用のクロージャを渡すという方法も無くはない。副作用のみを期待する場合には実は結構きれいにコーディングできる、、、のだがそれが罠。クロージャ渡しをしたくなる誘惑は主に呼び出し側と呼ばれる側の両方のコードが頭に入っている場合に発生する。例えば、関数Aが関数Bを呼び出し、関数Bの代替経路XとYで個別に処理する場合、代替経路の結果がXであるかYであるかの条件分岐を行ってから処理を行う必要がある。しかし、クロージャを渡してしまえば関数A側は処理を渡してそれでおしまいになる。しかし、クロージャが呼ばれたかどうかの保証は得られないし、クロージャの処理の結果に応じて行うべき処理を後付けしようとしたらクロージャから値を入れるための変数を事前に作るなどの処理が必要となり、コードがかなりカオスになってくる。また、クロージャの処理が実行されるタイミングは関連コードを全てホワイトボックスで同時に把握しない限り判断できないため、クロージャが呼び出されるタイミングが問題になった時に、関連コードすべての coupling が成立してしまう。とかいろいろ問題があるわけだが、コードの実行を時系列で追いづらくなるクロージャは無条件で複雑さを導入するのでそれ以上のメリットが無い場所ではやるべきじゃないと思われ。

とりあえずの結論:

  • 関数を代替経路のあるユースケースに見立てる場合は、例外は使わず sealed class を使い、経路毎に異なる型を割り当てる。*2
  • 関数の戻り値を複数得たい場合や分解して得たい場合は destructuring を利用する。*3
  • 戻り値を判断して対応可能な処理はクロージャ渡しでやらない。

という感じですかね。

*1:hoge()側の戻り値の型をどうするのかはプロジェクトのポリシー次第でしょうね。data class Point3D(val x: Int, val y: Int, val z: Int) のようなクラスを作るのか、Triple<Int, Int, Int> のようにカスタムクラスは作らないで対応するのかなど。可読性や将来的な変更局所性はカスタムクラスのほうが高いと思われる。

*2:when による条件分岐では when を値として利用しない場合も val dummy:Unit = when のように変数に代入して、sealed class の型を網羅していない場合はコンパイルエラーになるようにする方法が結構使える。

*3: val (x, y, z) = hoge() のような感じ。

preconditions, postconditions and assertions

とりあえず思い付きをメモ。

前提として、ゲームやツールアプリ開発にて、バグが発生しても動き続けるよりバグの発生自体を防ぐという方向性を想定。

事前条件と事後条件チェックの面倒な点は、条件が満たせなかった場合に理想的には全部実行時エラーで落とせばいいのだが、性能要件等の充足を阻害する可能性がある点。なので、javaJVM オプションで -ea を付けるか付けないかなどの二択というような方式をアーキテクチャ設計に組み込むと普通に死ねる。

ということで、事前条件と事後条件やアサーションの扱いは、アーキテクチャレベルでガチガチに決め込んでおいたほうがいい気がする。

ちなみに、アサーションについていろいろな考えがあると思うが、以後、プロダクション中では実行されないものとする。

で、何を決め込んでおいたほうがいいのかという話だが、項目は結構多岐に渡るので、実際に採用する場合の例を考えてみる:

  • メソッドとコンストラクタには、事前条件と事後条件が考えられる場合は必ず設定する。
  • 事前条件と事後条件は、条件を満たさないこととバグであることが同一である場合のみを指す。*1
  • 事前条件と事後条件は、基本的にはアサーションとしては扱わず、実行時エラーで落とす方式を採用する。しかし、性能要件充足を阻害する場合はアサーション方式を採用する。
  • バグ時でも強引に実行継続するするような場合はアサーションと例外ハンドリングは併用はせず、バグ発生もユースケースとして通常の例外ハンドリングに含める。*2
  • (メソッドやコンストラクタの呼び出し階層に関して)自身の階層のコード内部で困らないものは条件に含めない。*3

上記のような決め事をした際のメリットとかその他考察:

  • 事前条件と事後条件が自然言語ではなくプログラミング言語で書かれることにより曖昧さがなくなる。
  • 事前条件や事後条件を記述すべきかどうかの判断が不要になる。*4
  • 事前条件と事後条件を常に考えることにより、設計ミスが減る。*5
  • アーキテクチャレベルで決まっているのでコードに統一性が出る。*6

その他考察:

  • 内部コードや信頼できるソースからの入力の場合はアサーションは必要ないという考え方もあるが、これって微妙な気がする。なぜなら、呼び出し側のコードがわからなければその判断できないはずであって、その判断ができるということは、呼び出し側のコードとの coupling を意味するので。*7

という感じで、とりあえずメモしてみたw

*1:例外処理や条件分岐などは含まれない。それらは通常のユースケースとして扱う。

*2:二重管理による変更管理コスト増と変更管理失敗時のリスクがあるため。

*3:例えば userName という String 型のデータや age という Int 型のデータが引数にある際に、UserName や Age クラスとしてバリデーション済みが保証されたインスタンスを扱うのが理想だが、現実的にはそこまでやることは少ない。そのような場合、ユーザー入力やファイルや外部システムから受け取るような場合は受け取る処理自身の責務としてバリデーション(この場合はバグチェックじゃないのでアサーションではない)が入るが、それ以外の場所で呼び出しがあるたびにバリデーションしていたらきりがない。っていうか、そこまでやるなら普通にクラス化すべき。クラス化しない場合は、バリデーションされてないと階層内のコード断片の視点で自身の使命を果たせなくなる場合は事前条件や事後条件を入れないようにする。自身の使命が果たせない場合の例としては、Int を受け取りそれで数字を割るような場合は非0の事前条件を入れるとかは必要。自身の使命が果たせる例としては、アプリとして年齢が10-100歳という前提の時に、10年後の年齢を計算するコードがあった場合、そのコード自身は10-100歳という範囲を知る必要が無いし計算にも影響が無いので、事前条件を設定する必要が無い。ちなみに、『影響が無いから便器的に無視するのであって本来はチェックしたほうがいいんだよね?』という問いにはNO。これは常に無視するのが理想。なぜなら、そのような意味的(semantic)な関連性を考慮していたら、それらのコードは意味的に関連を持ち、結合度が強まってしまう。この syntactic ではないが semantic な coupling というのが実はアプリ開発で最も保守性を低下させるもの。具体的には、10-110歳までの範囲にする仕様変更があった場合に変更箇所どうなるの?コンパイラは教えてくれないよ?コンパイルエラーにもならず実行時エラーにもならないバグになるよ?という話。

*4:絶対に書くというルールなので

*5:特にドメインモデル設計の後半で実コードを書きながら考えられるというのは大きい。

*6:例えば、実行しない、開発時のみ実行(デフォルト)、常に実行、というオプション付きの assert 関数を用意してそれを利用するとか、それを利用した際の例をアーキテクチャ設計書に入れるとか。アーキテクチャ設計書が例ではなく実際のコードを自己参照してしまえば設計書の記述が風化することも無いし(リリースの際に設計書を一字一句全部レビューするという前提が必要だが)。

*7:アサーションを行うということは他のどこかでチェックされているという前提なので、それならそもそも正しいデータであることを保証する interface を用意することによりアサーション自体が必要無くなるはず。例えば、住所の文字列フォーマットをチェックしたいなら、システムへの入り口で Address interface としてチェック済み文字列を用意してしまえば以降のチェックは必要無くなるはず。

引数について

ふと思ったのでメモ。

要約:インタフェースメソッドの引数はインタフェース設計の視点で決定されるべきであり実装の都合を持ち込んではいけないかもしれないしそうでないかもしれないがおいらはそう思うw。

※サンプルを見やすくするため getter/setter は使っていない。
※プリミティブ型ではなくバリデーション等を含めてクラス化すべきとか言う話も今回は省略。

引数としてオブジェクトを渡すコード例:

interface User {
    val id: Long? // <- キモいw
    val name: String
    val address: String
}

interface UserService {
    fun register(newUser: User): User
}

必要な値のみ渡す例:

interface User {
    val id: Long
    val name: String
    val address: String
}

interface UserService {
    fun register(name: String, address: String): User
}

上記の2例は必要な値のみ渡す場合のほうが完成度が高いような気がする。しかし、User のメンバが100個あったらオブジェクトを渡す方式を採用してしまうことが多いのではないだろうか。

おいらの現段階での所感としては、そもそもバラすかどうかという考え方自体が本末転倒で、インタフェースの視点で必要な情報だけ渡せばいいと思う。

  • ユーザー登録時に id が決定されるので id を渡すのはそもそもおかしい。非 null を入れられたら実行時エラーにしなきゃならんとか無駄に複雑。
  • 別に User という形で渡す必要が無い。UIからユーザーが入力した値を元にUserを作成する場合になどわざわざ User を作るというのがナンセンス。

User の要素が100個あるような場合に一部データのみ変更したい場合は setter/getter のある UserInfo を作りたくなる気もするが、kotlin なら以下のように default argument と named arguments で対応できる。*1

interface User {
    val id: Long
    val name: String
    val address: String
}

interface UserService {
    fun register(name: String, address: String): User
    fun update(id: Long, name: String? = null, address: String? = null): User
    // 他での変更を考慮するなら下記のようにしてもいいかも。
    fun update(user: User, name: String? = null, address: String? = null): User
}

では、UserService.update() を叩く前にバケツリレーが必要な場合はどうかというと、確かに User の要素が100個あったらそれをバケツリレーするのはつらいものがある。しかし、UserService の視点では、そんなことは全く関係ない。つまり、UserInfo とか作りたいなら勝手に UserService の外でやってくれという話。しかし、以下のようなコードがソース中に分散されても困る。

// name, address, ... と100個あったら結構辛いものがある(´・ω・`) 
userService.register(user.name, user.address)

だからと言って、UserService に実装のツケを払わせるのは本質的におかしい。

重複するコードを抑えたいのであれば、以下のような方法で対処が可能。

interface UserInfo {
    var name: String
    var address: String
    fun register(userService: UserService)
}

しかし、UserInfo が UserService に依存するのも変。ならば、以下のように extension を使えばよい。

fun UserService.register(userInfo: UserInfo) {
    register(userInfo.name, userInfo.address)
}

extension の置き場とかはどこでもいいけど、UserService に入れるのはインタフェースの抽象化が崩れるので本末転倒。例えば、UserService が domain model 用のモジュール*2に配置され、UserInfo が domain model の実装用モジュールに配置されているとしたら、上記 extension も domain model の実装モジュールに入るのが正解ですかね。

*1:全ての引数に対して適切なデフォルト値を設定する必要がある。null が常に適切とは限らない。

*2:kotlin 用語としての module。

package private についてふと思ったこと。

ふと思ったので自分用にメモしておく。思い付きなので深く考えずにとりあえず殴り書きするだけっす。

あるオブジェクト内部の情報を外部に開示したくない場合に、情報隠蔽は有効な方法だと思われる。しかし、特定の相手には開示したいという場合もあり、その場合には java なら package private が利用できる。

おいら的には今は kotlin でのコーディングが大半を占めるようになったので、internal は使えても package private は使えない。kotlin 転向後に結構この点は心に引っかかったし、コミュニティでもこの点のディスカッション(Kotlin to support package protected visibility)が続いている。(たぶん永遠に終わらないwww)

当初はおいらも package private があったほうが良い派の意見だった。しかし、少なくとも現時点では全く困っていない。むしろ package private は多くの場合に保守性を低下させる要因になると考えるようになった。

package private 導入を望む人たちの意見は基本的に以下のコメントに集約されていると思われ。

if i understand it correctly your suggestion implies that if i want to have 1 public class and 9 package protected that do the actual job in java, i have to create a separate project declare those 9 as internal and then have this project compiled and accessible using Maven or Gradle or whatever one uses for the DI?

Imho that's an overkill...

Note that if i do 10 classes in one file 9 private i wont even be able to test those 9.

I personally never saw any code working around package protected access using same package name... it will just differ from your regular package structure and will constantly remind devs "fix me this is a shitcode".

言ってることはもっともだし、確かにそういうケースは多いと思う。しかし、言語の自由度を高くしたいという要求であれば、極論を言えばアセンブラを選択するべきであり、java vm 限定ならバイトコードを直接書くべきという結論に行きつく。当然ならがそういう話ではないわけで、行きつく先は、自由度をどのレベルに設定するかという話になる。

言語仕様がどうあるべきかという話なら、その言語がどのような設計方法論を支持するものなのかを考える必要があるんじゃなかろうか。

話は逸れるが、上記以外のコメントで java 言語仕様にあるんだから入れろ的なニュアンスの話もあったが、Java 言語が発表された 1995年当時と今では普及している設計方法論が異なるので、互換性のために java vm 仕様を踏襲しろというのは良いが java 言語仕様をそのまま新しい言語に取り込めというのは暴論だと思われる。

話は戻るが、べき論や抽象論で討論しても平行線なので、上記コメントにある、1つの public class が 9つの package private class を参照しているというケースをどのように扱うかという点について考えるのがこのディスカッションの落としどころになるんじゃないかという気がする。

この点において、ディスカッションは平行線を辿っている。『module にすればいいじゃん』 → 『それは overkill だ』的な感じでw。

あらゆる視点で考えるほどおいらはこの問題について考える気はないし、おいら自身が困っているわけじゃないのでどちらが正しいかというのも正直どうでもいい。しかし、おいらが困っていない理由だけは自分用にメモしておく。

『1つの public class が 9つの package private class を参照している』というようなケースに関して、それをおいらは package private にしようと思わない。理由は以下。

  • public A と package private の B, C というクラスのグループと、public D と package private の E, F というクラスのグループがあるとして、それらのグループ単体で package private を利用できるとする。その際、A と D を同一パッケージにしたいが、A から E, F、D から B, C にアクセスさせたくないとしたら、package private では対応できない。このような設計の拡張が無いことを保証するには、プロジェクトのソースコードの永久凍結宣言をするか未来予知ができるエスパーを雇わない限り無理。つまり、現代の多くのプロジェクトで採用されるクラス設計方針との相性が悪いという話。おいら的には、kotlin 転向後に、package private が使えないため auto completion の候補が多くなってウザーとか思った時期もあったが、これは慣れた。そもそも、上記の破綻例を解決できるようなパッケージングシステムがあったとしたら、現行の package private 方式もウザーとなるはず。慣れの問題ですね。
  • もう一つの理由は、(っていうか実はこれが唯一最大の理由と言ってもいいのだが)、package private を利用したい場合の大半は、静的な設計と動的な設計をごちゃまぜにしてるんじゃないだろうかという話。静的な設計の例としては、Stack 上に push, pop メソッドを公開して内部データは隠蔽するというような場合。この情報隠蔽は必要で、情報隠蔽をしないと Stack が外部要因によって自身の使命を果たせなくなる。クラス自身が自分の使命を全うするのは自身の責務。では、クラスAが package private のクラスBとCを参照するというケースで package private を public にしたらカプセル化が崩れるのかという点については、これは否となる。クラスとして B や C が外部から静的に参照可能でも全然困らない。重要なのは、動的なオブジェクトとして A が B, C を隠蔽していることである。*1

蛇足となるが、最後の理由はクラス設計の一般論としても重要で、動的視点を用いてアクセス制御を行おうとすると、クラス単体を設計する際にアプリケーション全体の動的設計まで考慮する必要が生じ、しかも未来予知ができるエスパーを雇わない限り保守性が著しく低下してしまう。現状のおいらの指針としては、モジュール設計はクラス設計の前に一通り完了させ*2、モジュール内のクラスはデフォルトで internal とし、実際に外部から呼ばれる必要が生じた段階で public にし、private に関しても実際にそうする必要が生じた際に適用するという感じですかね。

追記

  • CodeComplete にも "Avoid friend classes" っていうのがありますね。C++ の話だけど java でいうところの package private と同様に考えていいんじゃなかろうか。

*1:具体的なモノで例えるなら、Car クラスと Engine クラスがあるとする。世界で初めての車で、Engine はその車用に作られた概念とする。現状では Engine が使いまわされるなんて思いもしないとする。この状態で Car を 運転者に対するモノとしてとらえた場合、Car が Engine をカプセル化し、同時に情報隠蔽が行われるのは妥当。現時点だと Engine は Car のみにしか使われないので、Engine は package private で扱われるのも妥当に見える。しかし、Engine というクラス自体が public であっても全く困らないので、package private にする妥当性は全体的な視点によるものになる。つまり、Engine のクラス設計をする際に、Engine 外(具体的には Car)の事情を踏まえて考えてしまっているということであり、概念レベルでは Engine が Car に依存しているということになってしまう(要は syntactically independent だが semantically dependent ということ)。ソースコード上では単方向の参照だが、概念設計レベルでは双方向参照になってしまっている。これは概念設計上致命的な問題じゃなかろうか。実際に、Engine を列車に適用しようとした途端に package private は破綻する。Car と Engine の例は極論だという反論も出そうなものだが、それなら自社内の自分のプロジェクトだけで使うおそらく一度きりしか使わない特殊な形式のテキストファイルを処理するエンジンだった場合はどうだろうか。このような場合でも、そのエンジンが将来的に拡張されるかどうかは未来予知能力がない限りわからないし、分からないということは package private にする妥当性が無い(むしろ設計レベルでの保守性を低下させる)ということに他ならない。

*2:一通り完了させるというのは完成版ができるという意味ではなく、もちろん後で手戻りする可能性は織り込み済み。そのうえで、トップダウンの視点でモジュール内の主要クラスの洗い出し程度までの詳細化の範囲内のベストエフォートで完成させるという意味。『イテレーションの中で詳細化するから今は大雑把でいいやw』とか考えたら後で死にます。