技術をかじる猫

適当に気になった技術や言語、思ったこと考えた事など。

JUnit をもう少し管理者にみやすくしてみた

今日紹介するのはこれ。
Scala でザクッと作ってみた。

github.com

制作時間は調べ物 6h 、実装 4h か…
まだまだ精進が足りない。

こいつは先日書いた、JavaDoc を XML で吐き出すメモ - 謎言語使いの徒然 と、JUnit の結果をマージして出力するツールだ。
サンプルこんな感じ。

$ java -jar ut_converter.jar javadoc/javadoc.xml ut > result.html
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.

で出てくるのがこんな感じ。

f:id:white-azalea:20170611130253p:plain

JUnit は Spec じゃ無いから、表現力が低い。
そのため、メソッド名は目安になるけど、それ単品ではテストレポートとして出すのが難しい。

そこで、Javadoc で細かくテスト内容書いておいたら、まとめて表示してくれるツールがどっかで要るなと。
2週間位前にリポジトリは作ってたけど、やっぱ平日に手を出す余裕ないね(汗

引数パースライブラリの SCOPT(3.6.0) 使ってみた

紹介するのはこれ

github.com

依存はこれだけ

libraryDependencies ++= Seq(
  "com.github.scopt" %% "scopt" % "3.6.0"
)

で、引数を格納するクラスをこんな風に用意して

import java.io.File

case class Config(javaDocXml: File, junitResultDir: File)

パース設定とかを用意する。

import java.io.File
import scopt.OptionParser

object ArgParser {

  val parser = new OptionParser[Config]("UTXmlConverter") {
    head("ut_converter", "1.0-SNAPSHOT")

    arg[File]("[JavaDoc XML path]")
      .required()
      .text("Path for XML file that using MarkusBernhardt/xml-doclet.")
      .action((f, c) => c.copy(javaDocXml = f))
      .validate(v => {
        if (!v.isFile) Left("Not a file.")
        else if (!v.getAbsolutePath.endsWith(".xml")) Left("Not a XML file.")
        else Right()
      })

    arg[File]("[UT XML dir path]")
      .required()
      .text("Path for unit test results directory path.")
      .action((f, c) => c.copy(junitResultDir = f))
      .validate(v => if (v.isDirectory) Right() else Left("Not directory."))

    help("help").text("print this usage text.")

    note(
      "This program requires XMLs that generated by MarkusBernhardt/xml-doclet (var 1.0.5)\n" +
        " and Junit test results XML files."
    )
  }

  def parse(args: Array[String]): Option[Config] = {
    parser
      .parse(args, Config(new File("docs.xml"), new File("ut_results")))
  }
}

そして、引数指定しないで実行すると…

Error: Missing argument [JavaDoc XML path]
Error: Missing argument [UT XML dir path]
Try --help for more information.

おーいいね。
help 読んでみても…

$ java -jar ut_converter.jar --help
ut_converter 1.0-SNAPSHOT
Usage: UTXmlConverter [options] [JavaDoc XML path] [UT XML dir path]

  [JavaDoc XML path]  Path for XML file that using MarkusBernhardt/xml-doclet.
  [UT XML dir path]   Path for unit test results directory path.
  --help              print this usage text.
This program requires XMLs that generated by MarkusBernhardt/xml-doclet (var 1.0.5)
 and Junit test results XML files.

オプション引数や、引数バリデーションも書けるし、なかなかいいですね。

sbt を jar にしてみた

つっても何の事は無い。

github.com

これ突っ込んだだけ。

project/plugins.sbt に下記を追加して

addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.14.4")

build.sbt でざっくり指定するだけ。

name := "Example"

version := "1.0-SNAPSHOT"

scalaVersion := "2.11.8"

mainClass in assembly := Some("net.white_azalea.Application")

assemblyJarName in assembly := "example.jar"

これして sbt assembly するだけで、target/scala-2.11/example.jar が出来上がる。
以上。

JavaDoc を XML で吐き出すメモ

やりたい事が何かと言うと、外部プログラムにおいて、Java クラスのリスト取得と、そのドキュメントを引き抜きたいと考えた。

しかしながら、単純に javadoc のコマンドだけ実行すると、HTML ファイルが出てくる。
これは JavaDoc コマンドの仕様であり、基本動作となっていて、中間ファイルなども出てきていないようだ。

要するに javadoc コマンドの中でパースやら構築やら全部やってしまっていて、その間にあるだろう Javaクラス構成とそのドキュメント構造を引っこ抜く事がこのままだとできない。
でも当たり前だが、同じことをしたい人なんて絶対いるだろうと…むしろ居ない訳がなかろうと思ったのだ。

そして散々悩んだところ、どうも JavaDoc には Doclet なる機構が存在するらしい。
この Doclet とは、JavaDoc で読み取ったデータを出力する際のデータ整形に使われるようだ。

つまりこいつに XML 生成の Doclet 食わせてやれば、XML が吐き出されると思われた。
で、探して見たところ案の定。

github.com

そりゃそーだよなと、むしろない訳ないよなと。

って事でやってみた。

続きを読む

くたばれカーゴ・カルト・プログラミング

何?

Wikipedia によると。

実際の目的には役に立たないコードやプログラム構造を儀式的に含めておくプログラミングのスタイルである。カーゴ・カルト・プログラミングは主に、プログラマが解決しようとしているバグか解決策のいずれかかまたは両方を理解していない場合に見受けられる。 他にも、スキルの低い、ないし新人プログラマ(または当面の問題を経験したことのないプログラマ)が、そのコードが何をしているか理解が足りないまま、またはもしかしたら新しい場所にも必要なのではないかと、別の場所から関係ない部分も含めてコードをコピーしてしまうことで発生する可能性がある。

ひいてはコピペ駆動開発

どうして

不要なコードは、ロジック的に余計な処理であるがゆえにバグを生み出す温床(しかも実装者がわかってない事が多く、余計にバグりやすい)であったり、本来行うべき処理を読み取る中でノイズ(雑音)となって、レビュー時間、もしくは保守時の工数を浪費してしまう。
性能(無駄な処理)、安全性(余計な処理によるバグ発生)、保守性(可読性の低下)全てに悪影響をもたらす忌むべきもの。

どうする?

大抵は書いてあることの意味を理解していない為に起こる。
これには大きく分けて二つのパターン、もしくは両方の場合がある。

  • コピペ元がそもそも煩雑で、追いかけ切れない
    • ボーイスカウトの原則を守られず、コードが秘伝のタレになっている
    • 色々検証してて、動いたからいいやとリファクタをサボった
    • 可読性に無頓着でともかく手続きを並べまくってそのまま
  • 実装者が元のコードを読む気がない
    • やる気が無い
    • 読んでる時間的余裕がない

時間的余裕がない場合を除き、コミット前に書いた部分を最初から見直すのが一番。
また、修正の場合でも関連箇所はもっと共通化分けできないかとか考えるのが良い(DRY原則ボーイスカウトの原則)。

「1行1行注視して読める」では足りない。
メソッド化と分離を繰り返すと、次第にメソッドの流れは単純な複数のメソッドコールになっていく。

結果として、「流し読みできる」レベルで抽象化しているのが望ましい。

蛇足

因みにレビュアーの観点でこれをやるプログラマのもっともあるある Top3 は

  1. 可読性に無頓着でともかく手続きを並べまくってそのまま
    可読性の重要性を理解していない。
    知らないだけのパターンが多く、丁寧に説明すれば大体わかってくれる。
    それでダメなら、スパゲティなコードの保守プロジェクトに突っ込んで、次に原理主義的に綺麗なコード(オープンソース)のカスタムを経験させるに限る。
  2. コードが秘伝のタレ
    特にプロマネがせっつくだけで品質に興味がない場合で発生しやすい。
    この場合、現場が整理したいって言っても大抵工数を理由に拒否されるか、「動いてるのに手を加えるなんてありえない」と拒否される。
    この場合は、バグ数を理由に、レビューの実施(にかこつけてリファクタしてしまう)。
  3. やる気が無い
    このタイプは大体2タイプ。どっちも「こんなに戻って仕事進まないなら覚えた方がマシ」と思わせたら勝ち。
    • もらう金一緒なのに苦労とかしたくない派
      → 超絶レビューバックで矯正。
    • そんなのしなくてもやってこれた派
      → 汚いコードに劣等感持つまで、リファクタのやり方を指摘。

1 クラス 1 機能、1 メソッド 1 処理の徹底

どういう事?

名前の通り、クラス設計において、1つの機能は1クラスとして用意します。
また、1機能の実現もまた手続きがありますので、それら手続きも基本的にクラス、メソッドとします。

さらに、手続きの中でもループして書き換えるとか、なんらかの値に変更するなどの「その操作そのものは機能と言えないが、しかし一つの手順ではある」というコードをメソッド化します。

要するにあらゆる手続きは全てメソッド化し、分離可能なら処理毎にクラス化し、クラス単位の処理内容を徹底して単一化します。

場合によってはクラス内にメソッドは 1 つ、行数が数行という場合も発生しますが、それでもやります。

なぜ?

病的なまでに処理を分離するということは、クラス/メソッドの役割がそれだけシンプルになる。
これにクラス名、メソッド名を適切に設定すれば、修正が必要な際にも対象のクラスを即座に発見することができるようになります。
また、処理が十分に分離されているということは、処理は十分に単純であるということ。

単純な処理ほど 再利用が行いやすくなります
KISS原則 も必然的に守れますね。

どうする?

これはコーディング時にメソッド化すべき物は何かを常に考えます。
例えば、ログイン処理なんかはこんな感じでしょう。

コード自体はサンプルコードです。コピペしても多分コンパイルできないかと

最初に、メソッドと処理内容をざっくり考えます。

public HTTPResponse getProfile(final HTTPRequest request) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

では実装を開始してみましょう。

@Autowierd
private AuthorityRepository authorityRepository;

public HTTPResponse getProfile(final HTTPRequest request, final HTTPSession session) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    Optional<Long> loginAccountId =
        session.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong);
    Optional<Authority> authority =
        loginAccountId.flatMap(id -> authorityRepository.getOneByAccountId(id));

    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

でもこれは要するに手続きで、目的はセッションをキーに権限情報を取得する事にありますから、メソッド化した方が余計な変数が増えずに済みます。

public HTTPResponse getProfile(final HTTPRequest request, final HTTPSession session) {
    // リクエストからログインアカウントID、ひいては権限を取得します。
    Optional<Authority> authority = this.getLoginAuthority(session);

    // リクエストから取得するプロフィールの ID を取得します。
    // 取得した ID をキーに、対象のアカウントを DB から取得します。
    // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
    // アクセスできる場合は表示用の画面を返します。
    // アクセスができなければエラー画面を返します。
}

private Optional<Authority> getLoginAuthority(final HTTPSession session) {
    Optional<Long> loginAccountId =
        session.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong);
    return loginAccountId.flatMap(id -> authorityRepository.getOneByAccountId(id));
}

セッションからアカウントIDを取得する事は、およそ共通の処理でしょう。
ならば utils サブパッケージを作って分離しておくべきでしょう。

package controllers.utils;

public class SessionManagement {
  public static Optional<Long> readLoginAccountId(final HTTPSession s) {
    return s.get(Constants.SESSION_ACCOUNT_ID).map(Long::parseLong)
  }

追加で

@Autowierd
private AuthorityRepository authorityRepository;

public Optional<Authority> getLoginAuthority(final HTTPSession session) {
    return readLoginAccountId(session)
        .flatMap(authorityRepository::findOneById);
}

*1

こうした分離を繰り返すと、いつしか処理が本来の「getProfile(プロフィール取得)」から外れて、個々の小さな処理となります。
これを適切なクラスに分離していくことで、再利用性の高いクラスが出来ていき、同じような事を繰り返す事がなくなっていきます。

蛇足ですが、見ての通り、汎用的な処理とは即ち極小の、業務ロジックに直接関係しない個々の処理である事がわかります。
これは全体を眺めただけで見通すのは難しい。

設計はトップダウンとボトムアップ - 謎言語使いの徒然 にも繋がる話となります。

小さく区切って整理する。これが保守性を上げる一助となります。

私なら、getProfile は私ならこんな書き方をさせますね。

public Response getProfile(final HTTPRequest request, final HTTPSession session) {

    // 権限を取得し、取れないようなら、未ログインとして蹴ります
    Optional<Authority> authority = sessionUtils.getLoginAuthority(session);
    if (authority.isEmpty()) return redirect(request, Constants.LOGIN_URL);

    // リクエストから取得するプロフィールの ID を取得します。
    return this.readProfileId(request)

        // アカウントが今のユーザ/権限でアクセスできるかどうかを判定します。
        .filter(targetId -> this.profiles.isAccessibleProfile(targetId, authority.get()))

        // 取得した ID をキーに、対象のアカウントを DB から取得します。
        .flatMap(targetId -> this.profiles.getByAccountId(targetId))

        // アクセス取得できる場合は、表示用の画面を返します。
        .map(prof -> renderProfilePage(prof))

        // アクセスができなければエラー画面を返します。
        .orElse(redirect(request, Constants.HOME).with(
            Flash.error(Messages.get(Constants.ERROR_FORBIDDEN))
        ));
}

*1:複数のクラス(今回ならDBアクセス用の Repository を DI で使用等)に依存する場合、static なメソッドには切りにくいでしょうから、共通処理を含むインスタンスクラスを設計するのも良いでしょう。

オブジェクトの状態は作らない

ここで言う状態とは、クラス変数と振る舞いの状態を意味します。
例えば、クラス内の特定のフラグが true となったらメソッドの挙動が変わるなどです。

状態とは?

Java で例えれば、Closable インターフェース実装クラスなんかがそれに当たります。
リソースの解放と言う意味では仕方ないのでしょうが、close() を実行したあとでメソッドを呼び出すと例外を吐き出すようになります。

リソースを解放した後にアクセスできないのは当然の話ですし、close 後は使用不可(例外)を吐き出すだけまだマシと言うものでしょう。
一番怖いのは「動作が変わる」と言うパターンです。

例えばクラス自体が状態遷移図のようなものを持っている場合です。

状態遷移図(Wikipedia)

具体的にはこんな感じのクラス設計だ。

  • クラス動作に必要な値を、コンストラクタで初期化せず、setter 経由でしか設定できない。
  • メソッド呼び出しに順序が指定されている。
  • クラス変数の状態によって、getter 以外の public メソッドの挙動が変わってしまう。

何故状態が悪?

こうしたクラス設計とは、つまりクラスの内部的な動作を知らないと、意図した結果が得られない事を意味する。

オブジェクト指向の成り立ちが、もともと シミュレーション用言語 Simula なので、状態の変化と言うのはあって当然だったと思われる。
また、現実的にも自販機などのようにお金を投入された状態(ドリンク選択待機状態)など、状態を持つものは多い。

さらに、オブジェクト指向の本質、抽象化による現実の模倣(車とかがよく例に出されるが、それも状態だらけだ)としても、状態を是としている。
だが、いくつかの理由で、これはバグを産みやすくなる。

  1. (先に挙げたように)クラスの内部実装を理解していないと、正しく使えない。
    つまり、「この動作はこの機能の実装にも必要なんだけど…」と思った時に、そのクラスのコードを全て理解しないと使えないという事だ。
    状態がなく、クラス/メソッド名が確かなら、そもそもインターフェースだけで使い方がなんとなくわかる筈なのにも関わらずだ。
  2. 状態を持ったオブジェクトはテストしづらい。
    例えば、クラス内の状態を2変数で持ったとしましょう。すると、「テストすべき状態数=1変数で操作される状態数 x 1変数で操作される状態数」と倍数的に増えてしまう。
    まさかテストせずにリリースする訳にも行かないでしょう?
    テストコードを書かないとしても、結合テストでは、ここの分岐を動かすための操作、事前条件を整えるのが難しくなる。
    そうしたものは、結合レベルでは見逃す事も多い。
  3. 安心して他のメソッドに渡せない。
    オブジェクトに状態があり、それが書き換え可能だと、メソッド引数に入れて渡した時、その先で書き換えられてしまう恐れがつきまとう。
    それを避けるために、全ての手続きを追いかける必要性をプログラマに課してしまい、その行き着く先は1メソッドに全て記述する巨大な手続きだ。
    多くのプログラマが全力で嫌がるCOBOLバッチの世界へようこそ…。
  4. 常にコード内でオブジェクトの状態を確認しなければならない。
    メソッドによってオブジェクトの状態が変わるという事は、メソッドの呼び出し順序にすら制限しかねない不安定なものになる。
    次に待っているのは、その状態をフルコントロールするための状態確認処理の増加だ。
    それはビジネス的に本質的なものではなく、可読性を損う。

つまり、状態を持てば持つほど、それを使用するプログラムはそのコントロールに手間を裂く必要に迫られ、複雑化を招いてしまう。
これは保守性とは相容れない。

どう避ける?

私はこの問題に対し、 「クラス変数を使わない」 という方針をとる。
代わりに使用するのが 「クラス定数」 つまり、 コンストラクタによる初期化以外の値の変更を拒絶する 方針だ。

こうすれば、コンストラクタによってクラスの振る舞いは確定されるので、下記のようなメリットがある。

  • テストが書きやすい
  • 気軽にメソッド引数に渡しやすい
  • メソッドの呼び出し順序生が発生しにくい(DB など、外部入出力があるとまた少し違いますが)
  • 自分のメソッド内(もしくは自分のクラス内)で初期化したオブジェクトの振る舞いは固定なので、振る舞いを当てにしたコードを安全に書ける

欠点も挙げないと不公平でしょうから、欠点も挙げておきます。

  • 別の状態のオブジェクトを作るために、new が必要となってしまう。
    最近の Java/C# のランタイムは優秀で、変数を使わない場合の new コストはかなり低くなってきてはいる。
    しかし、ハードウェアの制約、ゲーム業界ほどの性能厨だと流石に考慮せざるを得ない。
  • 状態で物事を考えるという考え方を捨てなければならない。
    この考え方はどちらかと言えば「関数型言語」の思考に近い。
    慣れるには時間がかり、作るにも頭を働かせる必要がある。

しかし私はこの学習コストを必要悪だと考える。
理由は現在発生中のパラダイムシフトだ。
C++/Java は後発の方だが、F#, Scala, Rust, Kotlin などのように「オブジェクト指向 & 関数型」のハイブリッド化が時流だ。

いずれやらざるを得ないのなら、諦めてやるべきだ。
仕事が先細っていいなら構わないが…