大人数でプログラミングする時に気をつけていること（Java）

1人もしくは（すごく能力の高い人だけの）少人数での開発と、大人数での開発ではプログラミングに対して気にするべきポイントに差があるような気がしています。

ここでいう大人数の開発とは、「プログラミングがどれくらいできるのか、自分が把握していない人がコードを書く可能性がある環境」を想定しています。 ここに挙げているポイントは「規模感に関わらず常に気にすべきこと」も含まれていますが、大人数になると特に問題が大きくなりそうなものをピックアップしてみました。

また、レビューに対する工数をすごくたくさんかけられる環境など、開発現場によっては当てはまらないものもいくつか含まれていますが、本記事ではあまり気にせず思いついたものを羅列しています。 本記事では基本的に「レビューでつぶす」という解決方法はあまり考慮していません。粒度によりますが、本当に厳密なレビューフローがないと、どこかのタイミングで読みづらいコードがマージされることは防げないと思っています。

コード上でのデータの定義をサボらないようにする

例えばこのようなコードです。

public Map<String, Object> buildUser(String userName, int age, String country, String address1, String address2) {
    Map<String, String> address = new HashMap<>();
    address.put("country", country);
    address.put("address1", address1);
    address.put("address2", address2);

    Map<String, Object> user = new HashMap<>();
    user.put("name", userName);
    user.put("age", age);
    user.put("address", address);
    return user;
}

ここで作られる User には明らかになんらかの定義があります。 具体的には User は、Stringの name、intの age、String のフィールドをいくつか持った address を保持している可能性があります。他のフィールドも存在するかもしれません。 プログラム上で定義する場合、例えば以下のようになります。

public User buildUser(String userName, int age, String country, String address1, String address2) {
    return new User(userName, age, new Address(country, address1, address2));
}

public class User {
    public String name;
    public int age;
    public Address address;
    public User(String name, int age, Address address) {
        this.name = name;
        this.age = age;
        this.address = address;
    }
}

public class Address {
    public String country;
    public String address1;
    public String address2;
    public Address(String country, String address1, String address2) {
        this.country = country;
        this.address1 = address1;
        this.address2 = address2;
    }
}

• ※ そもそも buildUser いるのか？感がありますが、Map 版とインターフェースを揃えています。
• ※ ここではあくまで User の定義をすべき、という話をしたかったので、User をイミュータブルにすべきかどうか？については考えていません

なぜこのようにすべきか

少なくともこのコードを見た限りでは、User, Address クラスがない分前者の方がコード量が少ないです。 その上でなぜ後者のようにすべきかですが、ものすごく大雑把にいうとコードのボリュームが増えるにつれて、なにがなんだかわからなくなるからです。

例えば、User を組み立てるためのメソッドが buildUser だけではなく createUser や constructUser のようなメソッドがいつの間にか増えているかもしれません。 それらのメソッドが作成する User は、本当に同じ名前のフィールドを持っているのでしょうか？名前を表すフィールドが、name ではなく username とされている可能性はないのでしょうか？

例えば、私が User が name を持っていることを前提としたコードを追加するとき、その User に name というフィールドが存在することを、どのように確認すれば良いのでしょうか？ テストコードや注意深さによってこの問題を解消することは可能ですが、コード量が増えるにつれてどんどん面倒になっていくと思います。

人はすでに書いてあるコードを真似する

特に「大人数でプログラミング」をする場合の話ですが、多くの人はコードを書くときに、すでに書いてあるコードを真似する傾向にあると思います。

例えば「よくないのはわかっているが、面倒なので一旦 User を Map 型で扱う」という前提でコードを書いたときに、「 User の利用が複雑化したら User クラスを作ろう」と考えているかもしれません。

しかし「 User は Map として扱うんだな」と判断した人たちはいくら User の利用が複雑化しても、User クラスを定義してくれないかもしれません。 気がつくと、巨大で複雑なロジックが、定義のない Map の User を前提に組み立てられている可能性があります。

継承よりもコンポジション

正しい継承とは何か、どんな処理を継承元に持たせて、どんな処理を別のクラスに委譲すべきかについては色々な考え方があると思います。 また、Effective Java には継承についてサブクラスがスーパークラスの実装に依存するリスクから、継承を前提としていないクラスを継承することを禁止することが解決策として書かれています。

個人的にはそういった問題とは別に、「複数の実装が汎用的なクラスを継承している場合、多くの人にとって共通的なロジックは継承元に持たせたい気持ちになる」という問題があると思っています。

もしかするとこの継承には「もし継承元が持っているXの処理が増えたらXを別クラスのメソッドに分けよう」といった前提があるかもしれませんが、今後全ての変更をレビューできないのであれば、そのような意思が反映される保証はありません。気がついたら何が何だか分からないくらい継承元が膨らんでいる可能性は十分にあると思います。

個人的に最近見た範囲では、テストのコードに関して特に巨大なユーティリティ的な継承元クラスが作られやすいような気がします。 テストなので多少雑でもいいや、は気持ちとしてはわかるのですが、メンバー全体に「テストコードの設計は雑でも良い」という前提が作られてしまうリスクがあります。

責務を明示する

コメントには、何をするクラスか、と何をしてはいけないクラスか、を書くと良いと思っています。 これはもしクラス名自体がその2つを正確に表現していたとしても書くべきだと思っています。

クラスから責務を察するのは誰にでもできることではないことと、多くの人が母国語以外のテキストを見たときに思考が雑になる気がしているからです。

/**
 * XXXXX用のAPIクライアント
 *
 * <p>このクラスの責務は制限されており、2種類のメソッドのみを持ちます。</p>
 *
 * <p>A. 各APIのラッパー.<br>
 * このメソッドの引数は、clientおよびSwaggerによって生成されたリクエストクラスのみとしてください。<br>
 * このメソッドは渡されたリクエストでAPIを呼び出し、レスポンスを返却します。メソッド内でリクエストパラメータを操作してはいけません。</p>
 *
 * <p>B. デフォルトのrequest fixture生成.<br>
 * 各API呼び出しのためにデフォルトとなるパラメーターをセットしたリクエストのインスタンスを作成します。<br>
 * </p>
 *
 * <p>本クラスの利用例<br>
 * 1. 呼び出し元は、「B. デフォルトのrequest fixture生成」によってデフォルトのリクエストを取得します。<br>
 * 2. テストのために、必要なパラメータを1で取得したリクエストにセットします。（optional）<br>
 * 3. 「A. 各APIのラッパー」に2で作成したリクエストを渡してAPIを呼び出します。</p>
 */
public class XXXXXApiClient {

上記は以前このあたりに気をつけて書いたクラスの Javadoc です。 半年ほど経ってリソースの種類に対してクライアントの種類が30個ほどに増えていましたが、ざっと見た感じでは上記の責務から外れたコードが無さそうだったので、これが読みやすいかどうかはともかくコメントがある程度有効だったのかなと思っています。 コードを見たところ、Aのメソッド、Bのメソッド共にコメントごとコピペされてクラスが増えていったようでした。

これはクラスの特性に依存すると思いますが、特に横展開するようなものはテンプレート的にコピペして横展開できる（しやすい）と責務に対するブレが少なく済むのかな、とも思いました。 そもそもコメントを読んでくれないという話もあると思いますが、大切なのは可能性を上げるために何ができるか？だと思います。

メソッドの仕様を詳細に書く

• コードを読めば自明である
• シグネチャを読めばわかる

といった理由や、単純にめんどくさい、といった理由でメソッドの仕様を書き切らないケースがあると思っています。 しかし、「自明なのはある程度綺麗に書かれたコードだけ」だと思っています。呼び出し先のコードを常に全部見るのは正直めんどくさいです。 それを踏まえた上で、ある程度綺麗に書かれた自明なコードにも仕様を書くべきだと思っています。

なぜなら、仕様を書いていないメソッドを見た人が、コメントを書かなくても良い文化である、と判断した上で読むのが辛いコードを書く可能性があるからです。 「readXXX」という名前のメソッドが、read だけではなく何かを write しているとします。ものすごく極端な話な気がしますが、あるか無いかで言えば世の中にはこの手のコードは結構あると思います。 このメソッドを、コードを読まずに純粋に何かを read だけするメソッドだと思って呼び出すのは十分にあり得ることだと思います。

public User readUser(String id) {
    User user = readUserFromDb(id);
    writeUserReadCount(id);
    return user;
}

この時、以下のようにコメントに write していること記載していると、メソッドを呼び出す人がこのメソッドは read だけではなく write もするメソッドだと気づく可能性が上がると思います。

/**
 * DBからidにひもづくユーザーを取得します。取得できた場合、ファイルに当該ユーザーを読み込んだ回数を記録します。
 * @param id 取得対象のユーザーのID
 * @return idにひもづくユーザー
 * @throws FileNotFoundException 読み込み回数をカウントするためのファイルが存在しない場合
 */
public User readUser(String id) {
    User user = readUserFromDb(id);
    writeUserReadCount(id);
    return user;
}

スローする例外の種類について網羅的に書く、なども同じ話だと思います。 例えば、readUser という名前のメソッドがファイルへの書き込みに起因する例外をスローする可能性を考慮するのは、なかなか難しいです。

この話は、「責務に対してメソッドを綺麗に切る」や「メソッドに適切な名前をつける」ことよりも、自分が書いたコードに対して 「このメソッドが何をしているかをコメントに書く」ことの方が簡単 というところがポイントかと思います。

ただし単なる Getter に書く必要があるか？といった話もあるのでどこまで書くか、の閾値設定は難しいです。 個人的には全部書く、に踏み切ってしまうのも良いかと思います。 そもそも仕様を読んでくれないという話もあると思いますが、大切なのは可能性を上げるために何ができるか？だと思います。

まとめ

大人数でプログラミングをする際に、個人的に気をつけているパターンについてまとめてみました。

• そのコードと似たようなコードが増えていったときに極端に複雑さが増すかどうか？
• 今省略した手順が、今後増えるコードの複雑さに繋がらないか？

あたりを念頭においてコードを書くことで、コードの量が増えたときの可読性に繋がるのではないかと思います。

私からは以上です。