PG日誌

読者です 読者をやめる 読者になる 読者になる

PG日誌

主にc#の事を書いています

開始前チェックは確実に

c# Tips

大抵の処理は引数を受け取って開始される。そうでない場合、何らかの知識をオブジェクト自身から読み取って処理を開始するだろう。その時に受け取れるものが明確に定められる場合チェックを行うのは当然だ。例えば以下のようなコードがあったとしよう。

public class Sample {
    private string path;
    public Sample(string path) {
        this.path = path;
    }
    public void DoMove() {
        File.Move(this.path, this.getTargetPath());
    }
}

コンストラクタで受け取る前提条件をチェックしない場合、例外が発生するのはDoMoveメソッドとなる。発生する例外はArgumentNullExceptionやFileNotfoundExceptionとなるだろう。しかしこのクラスは呼び出し時に直接パスは指定していない。このタイミングで何らかの異常を受け取っても先ほどのコンストラクタに指定したpathが悪かったと気が付くには少々時間がかかる。しかしDoMoveを以下のように修正するのは間違いだ。

public class Sample {
    private string path;
    public Sample(string path) {
        this.path = path;
    }
    public void DoMove() {
        if(string.IsNullOrEmpty(this.path)) {
            throw new ArgumentException("path");
        }
        if(!File.Exists(this.path)) {
            throw new FileNotFoundException(path + " is not found.");
        }
        File.Move(path, this.GetTargetPath());
    }
}

短絡的に動くコードであればこれで充分だ。しかしこのクラスに対しDoCopyを追加すると問題が表面化する。

public class Sample {
    private string path;
    public Sample(string path) {
        this.path = path;
    }
    public void DoMove() {
        this.checkPath();
        File.Move(path, this.getTargetPath());
    }
    public void DoCopy() {
        this.checkPath();
        File.Copy(path, this.getNewFilePath());
    }
    private void checkPath() {
        if(string.IsNullOrEmpty(this.path)) {
            throw new ArgumentException("path");
        }
        if(!File.Exists(this.path)) {
            throw new FileNotFoundException(path + " is not found.");
        }
    }
}

これは非常にまずい。DRYの観点からも間違っている事は明らかだ。この様な取るに足らない事でローカルルールを作ってしまうと保守の時点で不具合の原因にもなりかねない。この場合、正しくはこうだ。

public class Sample {
    private string path;
    public Sample(string path) {
        if(string.IsNullOrEmpty(path)) {
            throw new ArgumentException(MethodBase.GetCurrentMethod().GetParameter()[0].Name);
        }
        if(!File.Exists(this.path)) {
            throw new FileNotFoundException(path + " is not found.");
        }
        this.path = path;
    }
    public void DoMove() {
        File.Move(this.path, this.getTargetPath());
    }
}

こうすることによってDoMoveはpath変数を必ず正しいことが保障される。万が一保障されないというならば、保障されるまで前提条件を追加し対応する。これで将来DoCopyやDoDeleteが追加された場合でもそれらのメソッドは安心してpathを参照することができる。

また、コンストラクタで例外を発生させることを是としている。c#なのでメモリリークする事はない。また、オブジェクトの一意性がpath変数によって担保される事を念頭に概念的に成立しない場合オブジェクトの生成自体をキャンセルするべきという設計上の判断に基づいている為である。

広告を非表示にする