俺のシステムがこんなにゴミクズなはずがない3

disってはきたものの、
仮に技術力や先見性を持った考え方だとか、そういったものを持ちあわせていても
どうにもこうにも期日やチーム状況や会社の方針によって
ゴミクズシステムは生まれてしまうことがあるのだと思います。
僕のような3年目のぺーぺーではなくて
仮に優秀なエンジニアであったとしても度重なる仕様変更やアクシデントによって滅亡されかけて
ゴミクズシステムを生み出してしまうことがあると思うんです。
というわけでゴミゴミとdisっていてもキリはない部分もあると思い
この3つ目のエントリでは1,2のような視点ではなくて
どうやったらこのようなゴミクズシステムを生まずに済むのだろうか、
そのあたりの技術面について書きたいと思います。
完璧とは言えないまでも、ゴミではなくて3秒ルールで口に運べるレベルのものであれば
ある程度運用していけるかなって具合です。

下記にポイントとなる要素をいくつか挙げました。
仮に自分が開発リーダー、動かせるメンバーが2人だと仮定します。
言語はJavaです。

開発環境の整備

とにかくローカル環境でアプリケーションの動作確認ができないといけません。
直面したことがない人にはびっくりかもしれませんが。
古いサービスで自アプリケーション以外の環境に依存しすぎていて
ローカル環境では動かすことができない状態である場合があります。
この依存関係を1つ1つ排除していって、とにかく開発者が各々ローカル環境で
開発を進められる状態に初期の段階で必ずもっていきます。

また、今後いつメンバーが増えるかわからないので
余裕のあるうちにwikiにローカル開発環境構築の手順を残しておきましょう。これ大事。

開発フローの整備

次に開発フローの整備ですが、これがチームの状況は刻一刻と移り変わっていくので
初期の段階でなかなか最善なフローを整備することは難しいと思います。
ただ今回取り入れた、git pushをhookしてjenkinsで自動ビルドデプロイを実行させる仕組みは
振り返ってみてもかなり開発効率を上げることができたと感じています。
スピードを考えてリリース前の調整段階に入るまではブランチを切らずに
masterブランチにガンガンコミットをしていく方針に決めたので
時にはビルドの通らないコミットをしてしまう人がいますが
jenkinsのビルド結果通知により失敗ビルドを検知することができました。
開発効率とシステムの安定性は対極の事柄に思えてしまいがちですが
そこをCIツール等でどこまでカバーできるかが重要です。
このあたりも開発の遅れで期日が迫っている時には手を付ける余裕など無い部分なので
初期の段階で仕組みを導入しておく必要があるかなと思います。
導入する前は大して変わらないだろうと思っていましたが、
導入してみると意外と効率が上がるのだなと実感できましたのでオススメです。
方法はググれば参考にできるかと思います。

End to Endのテスト

取り入れることができなかったテストコードを書く文化とテスト内容について。
これに関しては正直自分の知識が足りていないのであまり言えることもないのですが
YAPC2013でのnaoyaさんの発表がたまたまタイムリーだったのでとてもためになりました。

YAPC::Asia 2013 / Github によりバザールモデルへ
http://d.hatena.ne.jp/naoya/20130925/1380102703

モダンPerlリファクタリングのトーク（スライドうｐされています）で
リファクタリングのコツを紹介なさっていました。

今回の開発言語はJavaだったのですが、当然クローン開発によりクソコードを大量に引き継いでいるので
リファクタリングをするにも仕様を満たさないような変更をしないようにビクビクすることになります。
この恐怖はとーーーっても開発効率を妨げます。そもそもリファクタリングってそんな楽しい作業じゃないのに。
というわけで新機能に関してはテストコードを書くことにするとしても
既存の部分をクリーンコードへと変更すべくテストを書くのです。
じゃあどう書けば？というところで”End to Endのテスト”という言葉が出てきます。
中身はブラックボックスでも、このリクエストをしたらこのレスポンスがくることが正しいという風に
大きな括りで全ての入力に対する出力の期待値を決めて、そのテストを書くのです。
この扉を開けたら中で何が起こっているか全くわからないが必ず何も出てこない、
この扉を開けたら中で何が起こっているか全くわからないが必ずヤギが出てくる、というように。
このEnd to Endのテストの工数を確保し、メンバー2人にクローン元のソースコードを読んで
理解しながらひたすらに書いてもらうようにして、初期の段階で大きな括りの入力を網羅した
テストがオールグリーンになるようにします。
カバレッジなどを見えるようにしておけばテストを書くのがもはや楽しくなってくるかもしれません。
その間リーダーの自分は爆音でRed Hot Chili PeppersのCan't Stopを聴きながら
アプリケーションの全体を把握するように努め、どこに問題があるかを洗い出し優先順位をつけます。
このシステムはどのように改善されてどのように運用されていくのか
システムの最善な状態は何なのかを将来的な長い目線でひたすらに頭を悩ませて考えるのです。
これはクローン開発した機能の単なる機能改修の話ではなくて
開発・改修・運用等を考えたシステムの最善のあり方の話です。
これまた機能開発が始まってしまうと手が回らなくて
目の前のクソコードを直して機能追加することにどうしても注力してしまうから
初期の段階でやったほうがいいのではないかと思いました。

ソースコード改善

ここまでうまくいっているとすれば、全ソースコードにフォーマッターを適用して
一括で整った風に書き換えることもできます。
可読性を保つのに重要な様々な命名もある程度安心して変更していけます。
テストが通れば良いので、という安心感。素晴らしい。
pushしてビルドテストが失敗すればjenkins先生がお叱りしてくださいます。
ある程度対象の部分のリファクタリングが進んだら、
その部分のより詳細なテストコードを追加していくようにします。
こうして怖くないリファクタリングでクローン開発を進めていくのです。。

リファクタリング

クローン開発は、サービスとしては新規開発なのに
システムとしては古き悪しき名残のある状態から開発に入ることになるので
もはやリファクタリングが大半を占めていると言えます。
そのサービスを開発したころの流行りであったり
そのサービスを開発した人の技術力であったり
様々な要因で現在から見るとクソみたいな作りになっている部分が多々あります。
その中から工数・期日と相談して、改善できるところは改善しなければ運用でしんでしまいます。
良いコードを書くには、という点ではたくさん書籍が出ているので
それを参考にすれば基本的に問題はないのかなと思います。

このエントリで話してきたことはあまり具体的でない部分も多いので
この先も自分なりに解釈してベストプラクティスに近づけたら良いなとは思っています。
そしてここまでコードを一行も載せないという非エンジニアっぷりを見せつけたので
最後に申し訳程度にクソースコードを載せておこうと思います。

クローン開発の元サービスは、Javaのオブジェクト指向にバイバイして
ひたすらにMap<String, Object>を用いてデータをやり取りしていました。

以降のソースコードはイメージです。
これがControllerのクラス。

    @RequestMapping(value = "/item", method = GET)
    public String index(Model model) {
        List<Map<String, Object>> maps = new ArrayList<>();

        // 中略hogehogeしてる
        maps = itemService.getItemMaps(itemType);
        // 中略

        model.addAttribute("items", maps);
        return "item";
    }

何やらアイテム情報を取得しているようですが、Mapのせいでデータ構造が見えにくくなっています。
Mapにすることでviewで使う変数がいつでも容易に追加できる！みたいなメリットがあったのでしょうか。
ただでさえMapになっていて構造が把握しにくいのに、変数名やメソッド名まで
ぐちゃぐちゃだったりするので、それはもう把握するのに苦しみます。
とりあえず何も考えずコンソールにデータ構造を出力させてみたりします。

        System.out.println(new ObjectMapper().writeValueAsString(items));

ふむ、こんな構造なのか、と若干わかるようになります。
でもここでわかったつもりでいきなりリファクタリングをしてMapをBean化する行為をとると
結局あとでデータ構造がぐちゃぐちゃになってどうしようもないってことになりかねません。
少なくとも単にBean化するだけでどこでなんの値がセットされているか
わかるようにはなりますが、リファクタリングの途中で

        // 特別アイテムリストMap
        List<Map<String, Object>> specialItemMaps = new ArrayList<>();

みたいなものを見つけるとうわああああああああってなります。
そこらじゅうでMap、というか全部Mapだともうどの部分が共通のBeanとして使えて
どの部分はこっちのBeanを継承してみたいなデータ構造の関係を把握することが困難なのです。

なので、初め着手するときはこんな方法があるかなと思います。
ひとまずMapをまるっと囲えるBeanを作ってしまいます。
ただの命名です。lombok使っちゃいましょう。

import java.util.Map;
import lombok.Data;

@Data
@AllArgsConstructor
public class ItemMapBean {

    private Map<String, Object> itemMap;

}

このクラスを作るだけで

        List<Map<String, Object>> maps = new ArrayList<>();

が

        List<ItemMapBean> items = new ArrayList<>();

になります。ついでに変数名もリネーム。
どうでしょうか。2秒でできるのに、リファクタリングのとっかかりが作れた気がしませんか？

アイテム情報取得のメソッド側はこんな感じだとすると

    public List<Map<String, Object>> getItemMaps(int itemType) {
        List<Map<String, Object>> resultMapList = new ArrayList<>();

        // 中略 2重forとかでhogehogeしてる
                resultMapList.add(blackBox);
        // 中略

        return resultMapList;
    }

下記のように変わります。

    public List<ItemMapBean> getItems(int itemType) {
        List<ItemMapBean> items = new ArrayList<>();

        // 中略 2重forとかでhogehogeしてる
                ItemMapBean itemMapBean = new ItemMapBean(blackBox);
                items.add(itemMapBean);
        // 中略

        return items;
    }

このような第一段階を踏むと、可読性が皆無だった状態が解消されて
精神的にも少し楽にリファクタリングに取り掛かれると思います。
そこからは必要なBeanの種類や持つフィールドの内容を全体的に
把握できるようになるので随時Beanにフィールドを持たせるように変更していきます。
時間がなくなって第一段階のままのBeanが残ってしまうと
結局クローン元と同じ意味がわからないコードになってしまうわけですが
改善したい点が多々あって時間に追われる中で
深く考えて修正しなければいけない部分で手が止まってしまうよりは
即効性のあるリファクタリングを重視することも必要かなと思いました。

さいごに

ソースコードにかけるエンジニアにとって
クローン開発がどれほど苦痛なものか今回思い知らされました。
展開的には劇的に改善してやったぜ！というのを実現したかったのですが
こんなふうにやってこんなふうにダメだったよｗｗっていうのを
よくも悪くもさらけ出して書き残すことができてよかったかなと。

Ordinary

Today is the first day of the rest of my life.