SSブログ

コードレビューの視点 015 [コードレビューの視点]

意図しない継承に注意する

Java言語でクラスを設計する場合に、継承されることを想定していなくても、実際には継承可能なコードを書いてしまうことが多いです。たとえば、次のコードを考えてみてください。
public class Point {
    private final int x;
    private final int y;

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() { return x; }
    public int getY() { return y; }
}

このようなクラス宣言をした場合には、このクラスの使用方法は、1つではありません。単純にインスタンスを生成するだけでなく、クラスを継承して別のクラスを作成するという使用方法や、さらに、メソッドをオーバーライドして使用するなどができてしまいます。

ところが、コードレビューで、このようなコードを見かけたときに、「継承を前提とした設計なのですか?」と聞くと、「継承されることは想定していません」という返事が返ってくることがあります。Java言語では、継承できないように書くには、さらにクラスをfinal宣言するとかprivateのコンストラクタだけにするとかの手間をかけないといけないため、意図せずに継承可能なクラスを書いてしまうことが多いです。

しかし、継承される前提の設計でなければ、そもそも継承できないように書くべきです。コードレビューでは、その点を注意しなければなりません。特に、公開APIとなるようなクラスでは、なおさら注意する必要があります。

コードレビューの視点 014 [コードレビューの視点]

可視性に注意する

プログラミング言語の多くは、モジュール化を行うための何らかの言語仕様上の仕組みを持っています。たとえば、Java言語やGo言語であればパッケージ(package)をまとまりとして扱います。APIの観点で言えば、パッケージ外に公開されるのか、パッケージ内に隠蔽されるのかという違いとなります。

Java言語では、何らかのIDEを使用している人がほとんどだと思います。その結果、新たなクラスを作成する場合、デフォルトではpublicと宣言されたクラスが生成されたりします。

コードをレビューしてよく指摘するのは、「このクラスはパッケージ外から使用されることを想定しているのですか」ということです。つまり、「クラスをpublicと宣言しているのですから、外部から使用するという設計ですよね」という質問になります。

しかし、「パッケージ外から使用されることは想定していません」と回答されることがあります。パッケージ外からの使用を想定していないのであれば、そもそも、使用できないように最初から書いておくべきなのです。

可視性に注意することは、クラス宣言だけでなく、メソッド宣言にも適用されます。public宣言されたクラスだからといって、すべてのメソッドがpublicというクラスは少ないはずです。

コードレビューでは、設計上の意図がきちんと反映されているかを確認する必要があります。特に、その意図の記述が使用しているプログラミング言語により可能ならば、きちんとその言語の機能を使用すべきです。その一つが、可視性を正しく記述することです。