第4回　コードの分割―その4　Step2：共通処理のメソッド抽出

Step2：共通処理のメソッド抽出

まずは共通的に使用されている処理を別メソッドに抽出しましょう。リスト2③④の個所で「Elementの生成→要素の中身をセット」するコードが3ヵ所出てきます。

ここで前号の本連載で紹介した変数の「スコープ」に着目してみましょう。変数idNodeが使用される範囲は「node.appendChild」で追加されるまでのたった3行だけです（リスト3⁠）⁠。idNodeの生成を別メソッドに抽出すれば、idNode変数自体をなくしてしまうことができそうです。

node.appendChild(createElement(doc));
...
private Element createElement(
    Document doc, Division div) {
    Element idNode = doc.createElement("id");
    idNode.setTextContent(div.getId());
    return idNode;
}

リスト3　変数idNodeのスコープ

Element idNode = doc.createElement("id");   ┓
idNode.setTextContent(div.getId());         ｜idNodeが使用される範囲
node.appendChild(idNode);                   ┛

とりあえず、単純に「id要素を作成する処理」を抽出すると上記のようになります。Eclipseでリスト3の1行目と2行目を選択して、メソッドの抽出を機械的に行った場合はこのような結果になるはずです。一時変数のidNodeを別メソッドに移動したので、呼び出し元で無駄な一時変数が一掃できています。

ただし、このままだと「id要素にdivision.getIdの値をセットする」という決め打ちの処理になってしまいます。しかも引数にDivisionを渡しているので、組織要素専用のメソッドになります。ですので共通的に使えるように、「⁠呼び出しごとに変更したい部分」をメソッドの引数で渡すことにします。このメソッドでは「要素名」「⁠要素の内容」を呼び出しごとに変更したいので、これらを引数で渡すようにします。完成したコードはリスト4、図2のようになります。

リスト4　Step2：共通処理のメソッド抽出

...
public ActionResult index() throws Exception {
    ...
    int rowIndex = 1;
    Document doc = newDocument();   ―①
    Element rootNode = doc.createElement("data");
    doc.appendChild(rootNode);
    Element divisionsNode = doc.createElement("divisions");
    for (Division div: divs) {
        Element node = doc.createElement("division");
        node.setAttribute("index", String.valueOf(rowIndex++));
        node.appendChild(createElement(doc, "id", div.getId()));
        node.appendChild(createElement(doc, "name", div.getName()));
        divisionsNode.appendChild(node);
    }
    rootNode.appendChild(divisionsNode);
    ...
}
private Document newDocument() {                              ┓
    DocumentBuilderFactory factory =                          ｜
        DocumentBuilderFactory.newInstance();                 ｜
    DocumentBuilder builder = factory.newDocumentBuilder();   ｜
    return builder.newDocument();                             ｜
}                                                             ┛②
private Element createElement(
    Document doc, String nodeName, String textContent) {
    Element node = doc.createElement(nodeName);
    node.setTextContent(textContent);
    return node;
}
...

また、同様にリスト4①②のようにDocumentを生成している個所もメソッド抽出を行うことで、ローカル変数factory、builderを呼び出し元からなくすことができ、コードが読みやすくなります。

このように「何かを生成する」処理は、「⁠生成に必要な情報を引数で渡して別メソッドにする」ことでコードの可読性を高め、生成時に使用するローカル変数のスコープをメソッド内に収まるように小さくできます。

考察1：appendChildの呼び出しをcreateElementに持っていく？いかない？

リスト4 を見て、「⁠node.appendChild もcreateElementメソッドに移動して『要素を親要素へ追加する処理』も共通化するとよいんじゃね？」と思われた人もいるかもしれません。もしそのように修正すると、次のコードになります。親要素（parent）を引数に追加して、createElementメソッドで要素の追加を行っています。どちらが良いのでしょうか？

createElement(doc, "id", div.getId(), node);
...
private void createElement(Document doc, String
nodeName, String textContent, Element parent) {
Element node = doc.createElement(nodeName);
node.setTextContent(textContent);
parent.appendChild(node); // 外部の状態を変更!!
}

変更前のコードのcreateElementメソッドは、「⁠要素を生成するだけ」で、何かの「状態」を変更したりはしません。つまりメソッドを何度呼び出しても「外部に対して副作用がないシンプルなメソッド」になっています。

「要素を親要素へ追加する処理」をcreateElementメソッドで行うと、引数で渡される親要素の状態を変更してしまうため、「⁠外部に対して副作用がある」メソッドになります。副作用があるメソッドは「何が変更されるのか？」を意識して適切に呼び出す必要があります。

今回の例ならどちらでもあまり問題はないのですが、引数やフィールドの状態を変更するメソッドは副作用があるということを頭に入れておいてください。

考察2：createElementメソッドはユーティリティメソッドにしないの？

ユーティリティメソッド[11]にしてよいでしょう。「⁠繰り返し現れる処理」で「状態を持つ必要がないメソッド」はユーティリティメソッドの候補です。

Element elem =
DomUtils.createElement(doc, "id",div.getId());

Step2：共通処理のメソッド抽出

考察1：appendChildの呼び出しをcreateElementに持っていく？ いかない？

考察2：createElementメソッドはユーティリティメソッドにしないの？

考察1：appendChildの呼び出しをcreateElementに持っていく？いかない？