honeplusのメモ帳

上記の広告は１ヶ月以上更新のないブログに表示されています。
新しい記事を書く事で広告が消せます。

// 猫の身長に入力データを設定する
$cat->setHeigth($inputData);

以下は、最近触ってる実在のプロジェクトのソースを、プライバシーとか守秘義務とかに配慮した感じに簡略化した糞コードです。
（便宜上FuelPHP 1.8で作成。実物は違うのでFuelPHPでそれはおかしいとかは勘弁(--;）

なお、このプロジェクトには「直訳コメントは書くな、メソッド名などをちゃんとしてコメント無しでも分かるコードを書け」というコーディング規約が存在しています。

use \Model\Present;
class Controller_Present extends Controller {
    public function action_get_present($id) {
        $data = [];
        $data['result'] = Present::get_present($id);
        return View::forge('get_present', $data);
    }
}

はい、まあ分かりやすいシンプルなコードですね。
ぱっと見た感じで、処理内容が「プレゼントを取ってきて、それを画面に表示する」であることは一目瞭然です。
これなら、もちろん直訳コメントなんて要りませんね！
（個人的には、アクション名とかビュー名とか良くないと感じますが…まあ意味は通じるという観点で。）

が、実際にこのメソッドがやっていることは違います。Present::get_present($id) の中身を見てみましょう。

namespace Model;
class Present extends \Model_Crud {
    public static function get_present($id) {
        $data = [];
        $data['user_id'] = $_SESSION['user_id'];
        $data['present_id'] = $id;
        $present = Present::forge();
        $present->set($data);
        return $present->save();
    }
}

Σ (ﾟДﾟ;） そうです。このアクションは「プレゼントを受け取る」機能だったのです（怖

社内でも古いソースのようで、誰が書いたのかは定かではありませんが（ただしコピペされてたまに増える）、おそらくこのコードを書いたプログラマーは、「ユーザーがプレゼントをGETする処理だからget_presentだ」と思ってこのメソッドを設計したのでしょう…getという単語が他のプログラマーにどう認識されるかを考えないままに…。

このコードは、直訳コメント以前にそもそも糞コードであり、モデル内でセッションを見ていたりとツッコミどころはいくらでもあるのですが、少なくとも、直訳コメントがあればアクションを見ただけで異常に気付くことができました。

use \Model\Present;
class Controller_Present extends Controller {
    public function action_get_present($id) {
        $data = [];
        // プレゼントを受け取る
        $data['result'] = Present::get_present($id);
        return View::forge('get_present', $data);
    }
}

（上の例だと、直訳コメント以前にphpdocコメント書けって話になりますが、実際はもっと長いコードの途中に突然これが出てきます。っていうか、本当は直訳じゃなくて「// ユーザーに指定されたプレゼントを付与する」レベルで書いて欲しい。）


上は特に酷い例を抜き出していますが、ここまで酷くなくても、「英語のつもり」のメソッド名が、何を意味しているのか分からないことは日常茶飯事です。
逆に、メソッド名で意図が分かるコードを書いても、こういうのを書くプログラマーに正しく意図が通じるとは到底思えません。

読み手書き手ともに、英語ペラペラです、という特異な環境でない限りは、日本人プログラマーはちゃんと日本語のコメントも残しましょう、というお話でした。

PS. まあ糞コード書く人は、コメントも意味わからん糞コメント書くんですけどね…（虚ろ目