今回のお題

今回のお題は画像ファイルの一覧を表示するWebアプリケーションです。仕様は次のとおりです。

仕様

• ユーザは，FTPソフトを使用して画像ファイルをサーバ上の画像ディレクトリに直接アップロードする。画像ディレクトリの場所はWebアプリケーション直下の「webapp/images」（⁠注4）
• Webアプリケーションは，画像ディレクトリにアップロードされているファイルの一覧を表示する（図1①）
• 画像ディレクトリは，「⁠amimal（動物⁠）⁠」⁠「⁠food（食べ物⁠）⁠」⁠「⁠landscape（風景⁠）⁠」の3つのカテゴリに分けられている（図2）
• 画像ディレクトリごとに，ファイルサイズの合計を表示する（図1②）
• 画像ディレクトリごとに，ラベルとしてディレクトリの名称を表示する（図1③）

たいへんシンプルな仕様のWebアプリケーションですね。では，この仕様を満たすアプリケーションをいつもの3人組に書いてもらいましょう。

注4）

Webアプリケーション直下に動的に画像ファイルを配置すると，アプリケーションの再デプロイ時にデータが消えてしまったり，セキュリティ上の問題（JSPをアップロードされてしまうなど）が発生したりします。通常，このようなファイルは，適切にパーミッションが付けられたデータ用のディレクトリ（たとえば「/data/images」など）に置くことが多いと思います。今回はサンプルということでご理解ください。

Step1：ベタなコードで書いてみる

まずはベタに実装してみるのじゃ。

かしこ，かしこまりました，かしこ（注5⁠）⁠。けど，このくらいの簡単なアプリならいきなり抽象化して考えてもよくないっすかね？

ばかもの！ 筆者が困るじゃろ？「 まずはベタに書く⁠」⁠→「⁠それをリファクタリングしてい く過程を楽しむ」がこの連載のパターンじゃぞ。というわけでベタに書いてみるんじゃ。

画像ディレクトリが3つありますが，ベタに書くとリスト1のような感じになりますね。特に「ベタ」と強調して言わなくても，私はこういうベタなコードしか浮かびませんでした(>_<) メインの処理はstep1メソッドです（①⁠）⁠。3つの画像ディレクトリがあるので，それぞれのディレクトリのファイルの一覧とファイルサイズの合計を計算して，フィールド変数として画面に渡す形にしました。ビューはJSPでリスト2のように作成し，フィールド変数のファイル一覧とファイルサイズの合計を3ディレクトリ分ベタに出力しています。

@Path("/")
public class Step1Action extends Action {

    public ServletContext context;
    public File[] foodFiles;
    public File[] animalFiles;
    public File[] landscapeFiles;
    public long foodSize;
    public long animalSize;
    public long landscapeSize;

    public ActionResult step1() {                               ┓
        foodFiles =                                             ｜
            new File(context.getRealPath("/images/food")).      ｜
            listFiles();                                        ｜
        animalFiles =                                           ｜
            new File(context.getRealPath("/images/animal")).    ｜
            listFiles();                                        ｜
        landscapeFiles =                                        ｜
            new File(context.getRealPath("/images/landscape")). ｜
            listFiles();                                        ｜
        foodSize = sizeOfFiles(foodFiles);                      ｜
        animalSize = sizeOfFiles(animalFiles);                  ｜
        landscapeSize = sizeOfFiles(landscapeFiles);            ｜
        return new Forward("step1.jsp");                        ｜
    }                                                           ┛①

    public long sizeOfFiles(File[] files) {
        long totalSize = 0;
        for (File file : files) {
            totalSize += file.length();
        }
        return totalSize;
    }
}

<h1>Good Code Way 6 - Step1</h1>
<h2>
  Food Photos
  (<fmt:formatNumber
      pattern="0" value="${foodSize / 1024}"/>KB)
</h2>
<ul>
<c:forEach var="file" items="${foodFiles}">
<li>${file.name}</li>
</c:forEach>
</ul>
<h2>
  Animal Photos
  (<fmt:formatNumber
      pattern="0" value="${animalSize / 1024}"/>KB)
</h2>
<ul>
<c:forEach var="file" items="${animalFiles}">
<li>${file.name}</li>
</c:forEach>
</ul>
<h2>
  Landscape Photos
  (<fmt:formatNumber
      pattern="0" value="${landscapeSize / 1024}"/>KB)
</h2>
<ul>
<c:forEach var="file" items="${landscapeFiles}">
<li>${file.name}</li>
</c:forEach>
</ul>
</body>
</html>

なんかコードの重複が多い気がするなあ。DRY（注6）に反してない？

そうじゃのう。まずここで重複していそうなコードはcontext.getRealPath(" パス").listFiles()の部分じゃな。このコードは何をやっている部分かな？

対象の画像の一覧を取得してます。

そうじゃ，ここで「対象の画像の一覧を取得する」という処理が重複しているのが気になるのぉ。たとえば「JPEG画像のみ」表示するようになった場合，3つとも書き換えないといけなくなるじゃろ。単純に「コードの重複が悪」なのではなく，意味をきちんと考慮していない「処理の重複」が悪なのじゃ。

今のところ，「⁠JEPG画像のみ」という仕様ではないので，そこまでやっちゃうとYAGNI（注7に反しませんか!?

そうじゃな。先読みし過ぎはよくないわい。しかし，この部分は「対象の画像の一覧を取得する」処理だということが，ぱっと見てわかりにくいとは思わんか？

そうですね，特にcontext.getRealPathっていうメソッド（注8）を今回初めて知ったので，そっちに気が取られてコードの流れがわかりにくくなったように思えて気になっていました……。

メソッドに切り出すことで処理のブロックに「メソッド名」＝「名前（コードの意図⁠）⁠」⁠（⁠注9）を付けることができるので，コメントいらずの可読性の良いコードになることがあるぞ。普通のプログラマよ，ちょっとやってみてくれぬか。

注5）

意味がわからない人は「ハイキングウォーキング かしこかしこまりましたかしこ」で検索してください。

注6）

DRY（Don't Repeat Yourself，重複しちゃいけないよ⁠）⁠。システムのあらゆる場面で情報の重複を避けて，開発効率を上げたり，メンテナンス性を高めることです。書籍『達人プログラマー』で紹介されている考え方です。単純な「コードの重複」のみを指すものではありませんので，ここでの中級プログラマのDRYの使い方は若干微妙です。単純なコードの重複の排除は「Once and Only Once」と呼ばれます。DRYについて詳しくは特集1「現場で役立つDRYの基礎知識」（⁠9ページ）を参考にしてください。
参考：「DRY（Don't Repeat Yoursel）の意味を勘違いしてたかも」（⁠kwatchの日記）

注7）

YAGNI（You Aren't Going to Need It，どうせいらないって⁠）⁠。将来ばかりを考え過ぎても予想は外れることが多いので，今必要なことだけをシンプルにやっていきましょう，という考え方です。あとで本当にその機能が必要になっても，シンプルに保っておくことで機能が追加しやすいというわけです。
参考：「YAGNI ～ 予想でモノを作るな」（⁠悪態のプログラマ）

注8）

context.getRealPath。Servlet APIのServletContextのメソッド。このメソッドに「コンテキストのルートパスからの相対パス」を渡すと，ファイルの絶対パスを取得できます。サーブレットコンテナの実装によってはnullが返ることがあるので注意が必要です。詳細は次のAPIマニュアルを参照してください。
http://sdc.sun.co.jp/java/docs/j2ee/sdk_1.3/ja/techdocs/api/javax/servlet/ServletContext.
html#getRealPath(java.lang.String)

注9）

名前付けについては，本誌Vol.45の本連載第2回「名前付け重要。または，良いコードは良い名前から生まれるんです。」を参照してください。