メソッドの責任について考えたログ

エラー処理という名のガードレールが無いプログラミングを続けてきた。慣れないことを考えたので思案のログを残す。

条件設定

あるコレクションのメソッドにて、引数の識別子に一致する要素を探し、仕事をさせる。この時、引数が存在しないとき（undefined, null）デフォルトの値を使用する。

雰囲気つかみ

class Worker {
  digOilfield(): boolean{};
}

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithId(id: number): boolean{
  }

  takeDefaultId(): number {
  }
}

let workerCollection = new WorkerCollection();

digOilfieldWithId の引数に対して、どこまで不安になればいいのか考えていた。

思案1: digOilfieldWithId の中でデフォルトIDを取っちゃえ

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithId(id: number): boolean{
    if(!id){
      id = this.takeDefaultId();
    }
    this.workerList[id].digOilfield();
  }

  takeDefaultId(): number {
  }
}

let workerCollection = new WorkerCollection();

/* workerをかき集めたりする */

workerCollection.digOilfieldWithId(id);

一旦書いて、undefinedやnulllが問題じゃなく、Workerが逃亡しているかどうかのほうが重要だなと思った。

改善

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithId(id: number): boolean{
    if(!this.workerList[id]){
      id = this.takeDefaultId();
    }
    this.workerList[id].digOilfield();
  }

  takeDefaultId(): number {
    // 必ず有効なIDを返す
  }
}

let workerCollection = new WorkerCollection();

/* workerをかき集めたりする */

workerCollection.digOilfieldWithId(id);

「指定のIDに仕事をさせる」という名称なのに、居ないからという理由で他のIDに仕事をさせるのは、名前に嘘をついており”無責任”なのではと思う。

いると思ったID:10に仕事をさせたつもりが、じつはID:1が仕事をしてしまうのは使う側に対して無責任。*1

思案2: 使う側にちょっと手をかけさせる

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithId(id: number): boolean{
    this.workerList[id].digOilfield();
  }

  takeDefaultId(): number {
    // 必ず有効なIDを返す
  }

  existsWorkerWithId(id:number): boolean{}
}

let workerCollection = new WorkerCollection();

/* workerをかき集めたりする */

if( !workerCollection.existsWorkerWithId(id) ){
  id = workerCollection.takeDefaultId();
}
workerCollection.digOilfieldWithId(id);

確かに使う側が気を使って引数を厳選すれば回避できるかもしれないが、それは責務が違う。それにdigOilfieldWithIdがまた隙だらけになってしまった。

思案3: digOilfieldWithIdもっと怒れ

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithId(id: number): boolean{
    if( !this.workerList[id] ) {
        throw Error("そんな奴ァいねぇ！！");
    }
    this.workerList[id].digOilfield();
  }

  takeDefaultId(): number {
    // 必ず有効なIDを返す
  }

  existsWorkerWithId(id:number): boolean{}
}

let workerCollection = new WorkerCollection();

/* workerをかき集めたりする */

if( !workerCollection.existsWorkerWithId(id) ){
  id = workerCollection.takeDefaultId();
}
workerCollection.digOilfieldWithId(id);

メソッドの名前と異なることをしちゃあいけない。でも有効な値だけをもらえるかもわからない。いざ無効な値が来たときどうする。

もう怒るしかない。怒ることを使う側は知るべきだ。

思案4: 使う側は手間をかけたくない

class WorkerCollection {
  workerList: Worker[];

  digOilfieldWithIdOrDefaultId(id: number): number{
    if( !workerCollection.existsWorkerWithId(id) ){
      id = workerCollection.takeDefaultId();
    }
    return this.digOilfieldWithId(id);
  }

  digOilfieldWithId(id: number): boolean{
    if( !this.workerList[id] ) {
        throw Error("そんな奴ァいねぇ！！");
    }
    this.workerList[id].digOilfield();
  }

  takeDefaultId(): number {
    // 必ず有効なIDを返す
  }

  existsWorkerWithId(id:number): boolean{}
}

let workerCollection = new WorkerCollection();

/* workerをかき集めたりする */

workerCollection.digOilfieldWithIdOrDefaultId(id);

引数IDが無効ならばデフォルトIDを使うことを明言した。*2

この処理を使うということはデフォルトIDが使用されるかもしれないことを、使う側に認識させることができた。

とりあえずここで止め。

*1:そして副作用がID:1に起こってトラブルにつながるかもしれない

*2:流石に名前がおかしい気がする

ごらくらいふ

プログラミングしたりゲームしたり