前回のエントリ:アプリケーションハンガリアンを用いて徳丸本の「様々な列でのソート」のサンプルを改善してみた。を読んだ方から「わかりやすくなってなさすぎるw」という指摘をいただいたので補足します。
以下、いただいた指摘
伝えたかったことは、前回のエントリの繰り返しになりますが、外部から渡ってきた値を文字列連結でSQLに組み込んでいるのは臭うということです。
また、前回のエントリでは上記3行より関数の定義が先に記載されているのも分かりにくい原因だったかもしれません。
今回の議論で理解が深まった人がいれば幸いです。
今後もどこかの誰かに役に立つかもしれないことや、どこかの誰かが面白いと感じてくれるようなことを発信していきたいと思いますので、どうぞよろしくお願いします。
以下、いただいた指摘
- 意図は伝わったけどs/usというprefixは一般的じゃないからパッと見てsafe/unsafeだと分からなかった。
- SFromUSがsafe from unsafeの略とか分からなかった。
- Joelのエントリ読んでやっと理解した。
伝えたかったことは、前回のエントリの繰り返しになりますが、外部から渡ってきた値を文字列連結でSQLに組み込んでいるのは臭うということです。
$sort_columns = array('id', 'author', 'title', 'price');その臭いを消すために外部から渡ってきた値はunsafeとし、safeに変換してから使うようにしました。これにより安全であることを分かりやすくしたつもりです。
$sort_key = $_GET['sort'];
if (array_search($sort_key, $sort_columns) !== false) {
$sql .= ' ORDER BY ' . $sort_key;
}
$usSortKey = $_GET['sort_key']; // unsafeなprefixを持つ変数に格納Joelは「どこか1行のコードでその間違いを見つけられる」ことが利点だと言っています。
$sSortKey = SFromUS_SortKey($usSortKey); // unsafeな値をsafeに変換し、safeなprefixを持つ変数に格納
$sSql .= ' order by ' . $sSortKey; // safeなprefixを持つ変数を連結
$us = $_GET['name'];
これは正しい。
$usName = $us;
これは正しい。
$sName = $us;
これは間違っている。
$sName = $_GET['name'];
これは間違っている。
$sName = SFromUS($us);
これは正しい。
$sSql = ' order by ' . $_GET['name'];
これは間違っている。
$sSql = ' order by ' . $usName;
これは間違っている。
$sSql = ' order by ' . $sName;
これは正しい。
また、前回のエントリでは上記3行より関数の定義が先に記載されているのも分かりにくい原因だったかもしれません。
今回の議論で理解が深まった人がいれば幸いです。
今後もどこかの誰かに役に立つかもしれないことや、どこかの誰かが面白いと感じてくれるようなことを発信していきたいと思いますので、どうぞよろしくお願いします。