普段私がコードを書く時に気を付けていること

普段私がSAStrutsでアクションのコードを書く際に
気を付けていることをまとめてみました。


原則:

  • 既存コードを修正することなく、機能追加を実現する
    • 画面が追加されても、既存のメソッドに修正が入らず、メソッド追加で対応できること
    • ボタン追加などイベント処理が追加されても、既存のメソッドに修正が入らず、メソッド追加で対応できること
    • 初期化ロジックの無い箇所に後から初期化ロジックを追加する場合でも、クラスのインターフェースは変更しないこと
  • できるだけタイプセーフなコードにすること
  • メソッド・シグネチャにおける属人性を排除する


この原則を成し遂げるために、以下のコーディング規約を守る必要があります。
(オリジナルであって公式ではない。)


コーディング規約

  • 全てのアクションにindexメソッドを用意する
  • Teedaスタイルで入力系メソッドと出力系メソッドを分離する(※)
  • JSPを単独で使用しない。出力系メソッドとJSPはセットで1組と考える
  • たまたま1つのアクションに1つのJSPしか存在しない場合でも、複数のJSPが存在する場合と同じように命名にする

- 可能な限り、重複したJSPファイル名文字列をメソッド呼び出しに置き換える


Teedaスタイルによる入力系メソッドと出力系メソッドの分離ルール

  • 入力系メソッド:
    • 画面に入力されたデータを処理する
    • メソッド名は do + 任意の名前
    • 1ボタンにつき1つの単位で用意する
    • バリデーションを行う
    • input属性はjsp名ではなく、メソッド名を指定する
  • 出力系メソッド:
    • 画面に表示するデータを準備する
    • 1つのJSPにつき1つ用意する
    • メソッド名はJSP
    • 出力系メソッドは基本的にバリデーションは実施しない。
    • バリデーションエラーやカスタム戻るボタンで特殊な初期化処理になる時のみ toBack + JSP名 のメソッドを用意する


実際に、このやり方を適応したソースコードを見てみましょう。
まずは、ルールを適応していないバージョンです。
特定の仕様・局面に部分最適化されたソースコードになっています。


部分最適化されたコード

    @Execute(validator = false)
    public String index() {
        for (int i = 0; i < 5; i++) {
            Map<String, Object> m = new HashMap<String, Object>();
            m.put("id", i);
            m.put("name", "name" + i);
            mapItems.add(m);
        }
        return "foreachUpdate.jsp";
    }
                
    @Execute(input = "foreachUpdate.jsp", validate = "validateSubmit")
    public String submit() {
        return "foreachUpdate.jsp";
    }


次に、コーディング規約を適応して、全体最適化されたコードを見てみましょう。

    @Execute(validator = false)
    public String index() {
        return foreachUpdate();
    }
    
    @Execute(validator = false)
    public String foreachUpdate() {
        for (int i = 0; i < 5; i++) {
            Map<String, Object> m = new HashMap<String, Object>();
            m.put("id", i);
            m.put("name", "name" + i);
            mapItems.add(m);
        }

        return "foreachUpdate.jsp";
    }
    
    @Execute(input = "foreachUpdate", validate = "validateDoSubmit")
    public String doSubmit() {
        return foreachUpdate();
    }


見比べてみると、部分最適化されたコードの方があきらかに見通しが良いですね。しかし、注意して頂きたいのは、この部分最適されたコードは、この機能用にソースコードが最も簡潔に書けるように時間をかけてチューニングが施されたものであるということです。実際の実務では、多くの画面を作成するので、各画面ごとにその都度最適なチューニング方法を適応するのは、それなりの時間と高いスキルが必要になってしまいます。これでは思った以上に生産性があがりません。さらに、部分最適されたコードは、ゆとりが無いため、かなり仕様変更に弱いです。


例えば、画面やボタンなどを追加をしようとするだけでも、残念ながら動作している既存のメソッドに対しても修正が必要となってしまいます。テストコードの修正も考慮すると結構な作業です。一方、全体最適されたコードの場合は、画面やボタン追加のような仕様変更があった場合でも、既存のメソッドを修正することなく、新規にメソッドを追加するだけで対応可能です。(オープン・クローズ原則に似ています。)


他の例をもう一つ説明します。部分最適されたコードでは、input属性(バリデーションエラーがあった場合の遷移先パス)には foreachUpdate.jsp をセットしています。もし仕様変更があってforeachUpdate.jsp内にプルダウンリストが追加する場合、プルダウンリスト用のデータ準備処理が必要となります。この場合、実行メソッドでデータ準備処理を行ってから foreachUpdate.jspフォワードするようにプログラム構造を変更しなければなりません。indexメソッドはforeachUpdate.jspへの初期化処理を行っていますが、今回のケースだと input属性に index メソッドを指定することはできません。なぜなら、indexメソッド内で mapItemsに値を追加する処理があり、この処理はバリデーションエラー時に二重で呼ばれてしまうと不正なデータを作り込んでしまうためです。やはり、新しく実行メソッドを追加して、そのメソッド名をinput属性に追加する必要があります


やっかいなことに、SAStrutsは完全にタイプセーフなソースコードを書くことができません。(完全なタイプセーフにこだわると、生産性を下げてしまう設計思想と思われる。)コンパイルエラーの検出やEclipseリファクタリング機能が十分に活用できないので、その分コード修正のリスクは高くなることも考慮しなければなりません。


このように、上記の部分最適されたコードは、見た目はシンプルですが、生産性や保守性の面で課題があります。この課題をなんとかするために考えたのが先に述べたコーディング規約です。このコーディング規約を適応したコードはやや冗長になるものの「あまり迷わずに全体的に同じやり方が適応できること」や「コード修正に強い」といったメリットがあることを理解頂ければと思います。とは言いつつも、このコーディング規約は、まだまだ発展途上なので、フィードバック頂ければ幸いです。