check_xxx がなんでダメなのか - Yamashiro0217の日記
"check_xxx"みたいなメソッド名は避けようという意見には賛成。
だけど、流石にそれは例が悪い。
はてブのコメントでid:r-westが指摘しているように、いろいろな問題を詰め込んでしまってるから、「なぜその命名が拙いのか」という本題がかすんでしまっている印象。
"check_xxx の理由最初のだけ
"ならば、それに絞って書くべき。例え、"check_xxx 書くやつ、絶対他のこともそのメソッドでやるんだ
"としても。
以下、"check_xxx がなんでダメなのか
"を自分流に解説し直してみる。
元記事に則って、"check_user"というメソッド名を例に説明する。
元記事のコードは……Perl? PHP? どっちにしろ忘却の彼方なので、以下Cのコードという事で。
例えば、以下のコード片、一体何をやっているのだろうか?
enum User_Kind {
USER_KIND_UNKNOWN,
USER_KIND_NORMAL,
USER_KIND_SUPER,
USER_KIND_ADMINISTRATOR
};
struct User {
char *name;
unsigned int age;
enum User_Kind kind;
} user;
/*
* なんかいろいろ
* ながったるい処理が
* 在ったり無かったり
*/
if(check_user(&user)) {
/*
* なんかいろいろ
* ながったるい処理が
* 在ったり無かったり
*/
// (1)
} else {
/*
* なんかいろいろ
* ながったるい処理が
* 在ったり無かったり
*/
}うん、User型オブジェクトの「何か」をチェックして、それで処理を切り分けている……でも、何をチェックしてるの?
パッと思いつくのは次の2つか。
- userの各メンバが「正しい形式に基づいているか」をチェックする。
(nameに使われている文字種や最大長、ageの値が想定内の範囲か、kindの値が不正ではないか、等) - userの各メンバに設定した値に合致するユーザーが居るかどうかを(DB等から)チェックする。
他にも考えられる処理があるかも知れない。
仮に(1)の位置に"removeUser(&user)"とでも書かれていれば、後者の意味だったのだろうと予想(あくまで予想)出来るけれど、そこまで読み進めないと意味が読み取れないというのは、どうよ。可読性高いだろうか?
更に、1つだけならばまだしも二重三重と"check_xxx"な条件分岐がネストされていたらと考えたら……
え? 定義元を読めばいいじゃんって?
確かに定義元を読めばその関数が何やってるのかは分かる。理屈の上では。
でもさ、その定義元(この例の場合では"check_user")内に、更に"check_xxx"が在ったら? その定義も読むの? 更に更に"check_xxx"が在ったら? そうすると、どんどん読む必要が有る場所が増えていくよ?
で、読み終わって何やるメソッドか把握出来たら元の場所に戻って続きを……あれ?元の場所って何処だっけ?
// (各関数プロトタイプは省略)
// 略
if(check_user(&user)) { // 今読んでいる場所
// 略
}
// 略
_Bool check_user(const User *user) {
// 略
check_puyo(user);
// 略
}
Something check_puyo(const User *user) {
// 略
check_piyo(something);
// 略
}
Something check_piyo(const Something something) {
// 略
check_huga(something);
// 略
}
Something check_huga(const Something something) {
// 略
check_hoge(something);
// 略
}
Something check_hoge(const Something something) {
// 略
// ここまで読んで、やっとcheck_userが何やってたか分かる
}え? コメント書けばいいじゃんって?
// 各メンバと同じ値を持つユーザーデータが有るか調べる
if(check_user(&user)) {
// 以下略
それってつまりは、以下の「明らかな事をわざわざコメントに書く」のと一緒だよね。
// fooが0未満の時
if(foo < 0) {
// 以下略
明らかに無駄。
そんな無駄なコメント書くぐらいならば、分かり易い名前付けようよ、と。
if(exists_user(&user)) {
// 以下略
これならば、「ユーザーの何について調べたいのか」は自明だよね(もっと良い名前はあるかも知れないけれど)。
