一行のコメントから始まった思索
急ぎの要件変更依頼が舞い込んだ(私のプロジェクトでは常にそうだが):ダッシュボードの表示に使われていた2つのブランドデータに、3つ目のブランドデータを追加するという単純な要件。ビジネスロジックの変更はないため、2時間あれば簡単に対応できるだろうと考えた。しかし、コアロジック部分を確認した際に、以下のようなコメントが目に留まり、私の思考は天馬行空になった:
このコードを書いた開発者は、どのような過去の経験をしたのだろうか。しかし確実なのは、これは創造的な思考を持つ開発者によるものであり、自然と敬意の念が湧いてくる。
このコードは何度も変更が加えられ、開発者の心に波紋を広げたのだろうか。
あるいは、どのような要件が、開発者にこのようなコメントを残すほどのストレスを与えたのだろうか。
Gitのコミット履歴を確認すると、開発者はすでに他の部署へ異動しており、このコードがなければその存在を思い出すこともできなかった。コミット履歴は比較的整っており、大部分のコードは5月29日にコミットされ、5月30日はバグ修正のために小規模なコミットがなされていた。このコードは大きな変更を経験したわけではなく、要件は各ブランドの店舗を注文数でソートするだけのものだった(下図参照)。どう考えても大きな問題にはならないはずだ... しかし、開発者が残したコードを詳しく読むと、彼は自らに困難な課題を設定していた(これは控えめすぎる表現かもしれない)。少しの変更でも全体に影響を与える可能性があり、その結果として、成熟度が低く価値の乏しいコメントを残していたのだろう。最近読んでいたボブ大叔(Uncle Bob)の『クリーンコード』を思い出し、いくつか未熟な感想を持った。
コード品質の評価方法
『クリーンコード』の冒頭には有名な論断がある:コード品質を測る唯一の有効な基準は、WTF/min(毎分の「What The Fuck」の回数)だ。このような略語を見ると、その重要性を直感的に感じ、検索してみたが「標準的な」解説は見つからず、さらに高深なものに思えた。後になって偶然この略語が「What-The-Fuck」の頭文字であることを知り、この古典的な図を見直した瞬間、すべてが理解できたものだった。
この本は優れた書物だと感じ、多くのプログラマーが類似の書籍を読むべきだとさえ思っていた(例:マーティン・ファウラーの『リファクタリング』)。このような論述を目にし、この本が現実に即した内容であると感じ、まるで偉人との距離が縮まったかのようだった。過去の様々な経験や、最近歴史的なコードを修正した時の反応を思い返すと、WTFほど表現力のあるものはない。
冒頭で挙げたコメントに戻ると、当時の開発者も同じような反応をしていたに違いない。ただし、彼は自分自身を嘲っていたのか、あるいは問題は要件にあるのではなくコードにあると考えていたのかもしれない。したがって、WTF/minをコード品質を測る唯一の有効な基準とするには、限定詞を追加する必要がある:優れた開発者が発するWTFの回数こそが、真の基準なのである。優れたコードとは何かについては、『クリーンコード』や『リファクタリング』などの古典的な書籍で詳細に説明されている:
- 変数名からコメント、メソッド、クラス、モジュールに至るまで非常に詳細な規範
- 抽象化から境界、依存関係に至るまでの完全な方法论
- SOLID原則から、コンポーネント関連のCRP、CCP、LCPなどの原則に至るまで、理論から実践までの解説
正直に言えば、これらの書籍を読んだ後、私が以前書いたコードの多くがそのような思考を欠いており、多くのコードが非常に醜いと感じた。コードを書くことにおいて、まだ入門段階に過ぎないとさえ感じた。技術書を読むほど、自分の無知さを実感し、読書への意欲が湧いてきた。最近、技術書を読む合間に、数冊の人物伝記も読んだ:オッペンハイマーは天才と思われようと常に「こっそり」と勉強し、テスラは病床にあっても一心に読書に没頭し、私たちの剣士である銭学森はすべての空き時間を利用して読書し、1年で修士号を取得し、航空学と数学の博士号を取得し、最年少の終身教授となった(多くの栄誉を省略)... 彼らはすべて、自分の努力で読書し、思考し、実践を重ね、私たちが学ぶべき模範となった。最近の採用経験では、ほとんどの応募者が技術書を読む習慣がなく、特に技術書を読んでいない人が多いことに戸惑っている。中にはブログが書籍より役立つと考える人もいる。仕事が忙しいから役に立たないという人もいる... 彼らの中には、伝統的な企業で9時から5時まで働いている人もいれば、IT企業で996(朝9時から夜9時まで、週6日)で働いている人もいる。小規模な企業から来た人もいれば、アリババや快手から来た人もいる... その中で、一人の応募者の言葉が特に印象に残っている。最終段階で、彼に今も読書をするか尋ねたところ、彼はアリババに入社するために2年間非常に努力し、「Java仮想マシンを深く理解する」を2回読み、「高性能MySQL」「Kafkaを深く理解する」... すべてを注意深く読んだと答えた。アリババに入社後、これらの本は役に立たないと感じ、他の技術書を読んでいないという。私はアリババに行ったことがないので、何と言うべきか分からず、面接を終えた。私自身の経験では、かつての面接でもひどく突っ込まれたことがあり、同じような心境で「Java仮想マシンを深く理解する」「MySQL技術内幕:InnoDBストレージエンジン」「RocketMQ技術内幕:RocketMQアーキテクチャ設計と実装原理」「PaxosからZooKeeperへ:分散一貫性原理と実践」などの書籍を大量に読んだ。確かに、後の面接で冷静に対応できるようになった。しかし、読書を進めるほど、自分の知らないことが多いと感じ、読書で自分を充実させたいと思うようになった。そこで、システム設計やアーキテクチャに関する書籍の読書を始めた。今日に至るまで、『クリーンコード』を読んでも、10年間コードを書いてきたにもかかわらず、まだ多くのことを理解していないと感じる(笑、もしかしたら才能が足りないのかもしれない)。前述の偉人から、私の周りの人々まで、優れた人々は皆、読書の習慣があり、専門分野に関する大量の書籍を読んでいる。
最近、妻と読書について話し合った。私は週末に子供の世話をするか、ニュースをチェックするかして過ごしているが、最近のニュースやトレンドの多くは「栄養」に欠けるものだと感じた。また、短い動画が流行しており、3歳から70歳まで、地下鉄、公園、都市、田舎... どこでも見られることに話した。もちろん、私たちの両親も含まれる。この話をすると、自然と民族への憂いが湧いてくる(笑、少し心配しすぎかもしれない)。そこで私はニュースをやめたが、金曜日の夜、子供が寝てからは、軽い夜食を買い、古典的な映画を見る習慣は維持している。しばらくすると、1週間に10時間以上読書するのも普通に感じられるようになり、読書が生活の一部になった。以前のような「どれだけ読んだか、休憩が必要だ」という考えや、ニュースやテレビを見る気持ちもなくなった。今考えると、読書であれニュースや短い動画であれ、本質的な違いはなく、心が満たされていれば良いのだろう。
先ほどの面接に戻ると、面接の過程で読書をこれほど重視することが適切かどうかは分からない。しかし、技術書を読むことを好む人は、それほど悪くないはずだと信じている。
元のコードに戻る
冒頭のコメントの問題について、コードのリファクタリングプロセスを皆さんと共有したい。もし不幸にも元の開発者がこれを読んだとして、気にしないでほしい。ボブ大叔が言うように、すべてのプログラマーは専門的な視点からコードをレビューされるべきだからだ(笑、私自身もそれほど専門的ではないかもしれない)。要件は比較的単純で、2つのブランドの店舗を注文数でソートするだけだった。現在の要件は3つ目のブランドを追加し、店舗情報にはブランド属性がある。
もしこの開発者が『クリーンコード』を読んでいたなら、彼はコードがクリーンになるための4つの原則を理解していたはずだ:
- すべてのテストを実行する
- 重複をなくす
- プログラマーの意図を表現する
- クラスとメソッドの数をできるだけ減らす
そうすれば、ソートアルゴリズムを分離し、ビジネスロジックから切り離し、大量の重複を避けられたはずだ。
もし彼が「DRY(Don't Repeat Yourself)」の原則を深く理解していたなら、多くのConsultationOrderRankオブジェクトの初期化はなく、データがある場合とない場合を個別に処理することもなかっただろう。
もしこの開発者が『クリーンアーキテクチャ』を読んでいたなら、抽象に向き合うこと、具体的な実装に向き合うのではないことを理解していたはずだ。
彼は具体的なブランド1やブランド2に向き合うのではなく、ブランドという概念に向き合ってプログラミングしたはずだ。
// タスク変更後のクリーンな実装
// 空の注文リストの場合の処理
if (CollectionUtils.isEmpty(orderList)) {
List<CfgStore> allStores = cfgStoreService.getAllStores();
Map<Integer, List<ConsultationOrderRank>> brandResults = new HashMap<>();
// 各ブランドごとに店舗リストを作成
for (Integer brandType : getSupportedBrands()) {
List<CfgStore> brandStores = allStores.stream()
.filter(store -> store.getBrandType() == brandType)
.sorted(Comparator.comparingInt(CfgStore::getStoreId))
.collect(Collectors.toList());
List<ConsultationOrderRank> rankList = brandStores.stream()
.map(store -> createEmptyOrderRank(store))
.collect(Collectors.toList());
brandResults.put(brandType, rankList);
}
return buildOrderResponse(brandResults);
}
// 注文データがある場合の処理
Map<Integer, Map<Integer, Long>> brandOrderCounts = calculateOrderCounts(orderList);
Map<Integer, List<ConsultationOrderRank>> brandResults = new HashMap<>();
// 各ブランドごとに処理
for (Integer brandType : getSupportedBrands()) {
List<CfgStore> brandStores = storeList.stream()
.filter(store -> store.getBrandType() == brandType)
.collect(Collectors.toList());
List<ConsultationOrderRank> rankList = brandStores.stream()
.map(store -> createOrderRank(store, brandOrderCounts.getOrDefault(brandType, Collections.emptyMap())))
.collect(Collectors.toList());
// 注文数でソート
List<ConsultationOrderRank> sortedList = rankList.stream()
.sorted(Comparator.comparing(ConsultationOrderRank::getOrderNum).reversed())
.collect(Collectors.toList());
// 同じ注文数の場合の順位付け
assignRanks(sortedList);
brandResults.put(brandType, sortedList);
}
return buildOrderResponse(brandResults);
以下はヘルパーメソッドの実装例です:
// ブランドタイプの一覧を取得
private List<Integer> getSupportedBrands() {
return Arrays.asList(0, 1, 2); // 0: ブランドA, 1: ブランドB, 2: ブランドC
}
// 空の順位オブジェクトを作成
private ConsultationOrderRank createEmptyOrderRank(CfgStore store) {
return ConsultationOrderRank.builder()
.storeId(store.getStoreId())
.storeName(store.getNameAlias())
.orderNum(0)
.sort(0)
.build();
}
// 注数を基に順位オブジェクトを作成
private ConsultationOrderRank createOrderRank(CfgStore store, Map<Integer, Long> orderCounts) {
long orderCount = orderCounts.getOrDefault(store.getStoreId(), 0L);
return ConsultationOrderRank.builder()
.storeId(store.getStoreId())
.storeName(store.getNameAlias())
.orderNum((int) orderCount)
.sort(0)
.build();
}
// ブランドごとの注文数を集計
private Map<Integer, Map<Integer, Long>> calculateOrderCounts(List<HeOrder> orders) {
Map<Integer, Map<Integer, Long>> result = new HashMap<>();
// ブランドごとにグループ化
Map<Integer, List<HeOrder>> brandGroups = orders.stream()
.collect(Collectors.groupingBy(HeOrder::getBrandType));
// 各ブランド内で店舗ごとに注文数を集計
for (Map.Entry<Integer, List<HeOrder>> entry : brandGroups.entrySet()) {
Integer brandType = entry.getKey();
Map<Integer, Long> storeCounts = entry.getValue().stream()
.collect(Collectors.groupingBy(HeOrder::getStoreId, Collectors.counting()));
result.put(brandType, storeCounts);
}
return result;
}
// 同じ注文数の場合の順位付け
private void assignRanks(List<ConsultationOrderRank> rankList) {
if (rankList.isEmpty()) return;
int currentRank = 1;
int sameCount = 0;
for (int i = 0; i < rankList.size(); i++) {
ConsultationOrderRank current = rankList.get(i);
if (i > 0 && current.getOrderNum() != rankList.get(i - 1).getOrderNum()) {
currentRank += sameCount + 1;
sameCount = 0;
} else {
sameCount++;
}
current.setSort(currentRank);
}
}
// 注文レスポンスを構築
private Order buildOrderResponse(Map<Integer, List<ConsultationOrderRank>> brandResults) {
return Order.builder()
.brandAList(brandResults.getOrDefault(0, Collections.emptyList()))
.brandBList(brandResults.getOrDefault(1, Collections.emptyList()))
.brandCList(brandResults.getOrDefault(2, Collections.emptyList()))
.build();
}
このようにリファクタリングすることで、コードは以下の点で改善されます:
- DRY原則に従い、重複コードを排除
- 単一責任の原則に基づき、各メソッドに一つの責務を割り当て
- 拡張性を高め、新しいブランドの追加が容易に
- 可読性を向上させ、メンテナンス性を改善
これにより、私たちはプログラミングにおける少年兵の規則を実践しました:キャンプを離れるときには、来たときよりもきれいに整頓しておきましょう。
優れたプログラマーになるために!