- ベストアンサー
冗長コードのfunction化と汎用化
- GETで検索文字列（$_GET["q"]）が入っていたら、その文字列を太字に変換しておく、その作業を複数のカラム（$myval1～5）を対象に施したい。
- 冗長な繰り返しコードのfunction化のメリットを理解し始め、上記の作業をfunction化したい。
- 検索文字列がスペースで区切られていた場合、上記の処理を適用する方法がわからない。
- みんなの回答 （7）
- 専門家の回答
質問者が選んだベストアンサー
- ベストアンサー
いろいろ機能を付け加えて、更には（せっかく他のDBに書き換えが容易なPDOを使っているので）DSNの部分をメモリー上に作成するSQLiteに書き換えて、簡単にコピペで動くようにサンプル作ってたんですが…LIKE検索用のエスケープ方法がMySQLとSQLiteで全然違ってかなり困ったので結局MySQLに途中から戻しました。そこからのデバッグは行っていません、多分大丈夫だとは思いますが… サンプル http://pastebin.com/TTDkVmf3 全角英数字は半角英数字に変換して、大文字小文字を区別する検索が出来るようにstrtrではなくpreg_replaceを使う方法に戻したのですが、実際のDBのデータってどうなってるんでしょうか？全角英数字であればそもそもそういった検索が不可能なので、strtrを使う方法に戻してもらったほうがいいと思います。
その他の回答 (6)
>> PDOにする場合は、↓のように「こちょこちょ」と小手先の変更で済むのでしょうか？？ すいません、質問にちゃんと答えていませんでした（汗 はい、このコードで問題ないですよ。ちゃんと動くと思います。 Mysql関数で例外オブジェクトを使わずに無理やり書いてみた例 （毎回エラーチェックしてるのがバカバカしく思えるでしょう） http://ideone.com/OcbkDs 一応フェッチスタイルについて補足しておきます。 ・mysql_fetch_array / PDO::FETCH_BOTH(何も指定しなかった場合のデフォルト) 連想配列と数字添字配列の両方を取得する ・mysql_fetch_assoc / PDO::FETCH_ASSOC 連想配列のみを取得する ・mysql_fetch_row / PDO::FETCH_NUM 数字添字配列のみを取得する 今回は「好きな食べ物<?=$i+1?> 」と書きたかったので、3番目を採用しています。
お礼
> Mysql関数で例外オブジェクトを使わずに無理やり書いてみた例 > （毎回エラーチェックしてるのがバカバカしく思えるでしょう） 確かにmysql関数では冗長になりますね、せっかくの機会をいただきましたので、今後こそ ここで歯を食いしばって（？）PDO化を果たしてしまいたいところですが http://ideone.com/ePy31D ↑このご提示サンプルコードで、 > $list = array( > array('とうふ', 'たまごやき', 'めろん'), > array('みかん', 'サラダ', 'みそ汁'), > array('たまごとうふ', 'たまごやき', 'チョコ'), > ); > $h = new Highlighter('とうふ たまご '); ここの処理がよくわかっていません（配列が苦手で、、、） そこで、まことに、まことにあつかましい話で心苦しいのですが、以下が再現できれば、当方本番環境にも組み込むことができそうです。 もしお時間あれば、ご提示サンプルコードの延長に組み込みお願いできますでしょうか？ (1) ご提示のコードを文末のサンプルデータ（→文言コロコロ変えてすみません）で、 (2) ステートメントをwhileループ+フェッチ系のメソッドで回し、 (3) 検索フォームに「野菜 栽培」をSubmitされたのを$_GET['q']で受け取り、以下のSQL文を構築する（参考：http://oshiete.goo.ne.jp/qa/8310929.html） $sql = "select * from magazine where"; if ($_GET["q"] != '') { $q = mb_convert_kana(trim(htmlspecialchars(stripslashes($_GET["q"]))),"AKNRV","UTF-8"); $qs = array(); foreach (preg_split('/(\s| )+/', $q) as $word) { array_push($qs, "concat(Title,' ',Author) like '%$word%'"); } $sql .= ' and (' . implode(' and ', $qs) . ')'; } ※ 結果、出来上がりの$sqlは↓となる select * from magazine where (concat(Title,' ',Author) like '%野菜%') and (concat(Title,' ',Author) like '%野菜%' and concat(Title,' ',Author) like '%栽培%') (4) 検索キーワード「野菜」or「栽培」にマッチする部分を太字でハイライトした以下のHTMLを出力 <ul> <li>2 2011年版＝「<b>野菜</b>のハウス<b>栽培</b>（ジャーナリスト：山本花子・作）」 <li>3 2012年版＝「<b>野菜</b>の育成（<b>野菜</b><b>栽培</b>協議会：佐藤次郎・作）」 </ul> (5) サンプルデータ（MySQL） CREATE TABLE IF NOT EXISTS `magazine` ( `Code` int(5) unsigned NOT NULL auto_increment COMMENT '管理コード', `PubYear` int(4) default NULL COMMENT '発行年', `Title` varchar(900) default NULL COMMENT 'タイトル', `Author` varchar(600) default NULL COMMENT '著者所属と氏名', PRIMARY KEY (`Code`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=5 ; INSERT INTO `magazine` (`Code`, `PubYear`, `Title`, `Author`) VALUES (1, 2010, '盆栽の育て方', '盆栽協会：鈴木太郎・作'), (2, 2011, '野菜のハウス栽培', 'ジャーナリスト：山本花子'), (3, 2012, '野菜の育成', '野菜栽培協議会：佐藤次郎'), (4, 2013, '植木の栽培と剪定のやり方', '名古屋農業大学：高田三太');
あーなるほど、雑誌の記事のハイライトですか。結構重要な機能ですねこれは。 >> $col['fruit']の部分（連想配列、と呼べばいいのでしょうか） >> の書き換え方がわからず、PDOへ乗り換えきれていないのが本当のところです。 取得される連想配列の形は比べられた2つで全く同じです。Mysql関数ではwhileループで一つずつ取り出していく書き方しか出来ませんでしたが、PDOクラスでは様々な書き方が出来ます。 「SELECT の結果を取得する」 http://qiita.com/mpyw/items/b00b72c5c95aac573b71#2-18 ここで詳しく解説していますのでもう一度ご覧ください。 ・ステートメントをwhileループ+フェッチ系のメソッドで回す ・ステートメントをTraversableインターフェースを利用してforeachで回す ・一気に配列として全件取得する ここでは分かりやすいように3番目を採用しておきました。 >> PDOにする場合は、↓のように「こちょこちょ」と小手先の変更で済むのでしょうか？？ PDOクラスに変えたからといって、コード量が増えるわけではありません。むしろ減ります。正直、Mysql関数を使う場合の方が大変だと思います。 「mysql_connect」「mysql_select_db」「mysql_query」といった関数は何らかの原因でコールに失敗する可能性があり、そのときに返り値がFALSEになるので、それを検知して例外処理を書くのが本来のコーディングです。現状のコードだと、接続に失敗したらそのままwhileで取り出しているところまでWarningを大量発生しながら突き進んでしまいます。 一方PDOクラスでは、このような失敗が発生したときに、自動的に「PDOException」として例外オブジェクトをスローしてくれて、Catchブロックにジャンプさせることが出来るのです。このメリットは非常に大きいでしょう、オブジェクト指向の大きな強みの一つです。ideone.comに提示したサンプルでもドライバが見つからないという名目の「could not find driver」という例外がスローされていることが確認できるでしょう（実際の環境ではちゃんと動くので問題ないですが）。
お礼
To_aru_Userさん、お世話になっておりますm(_ _)m また、毎度詳しい解説大変勉強になります。 PDOでの接続など以前参考になるまとめサイトを提示いただき 見直しましたが、これ、To_aru_Userさんご自身による記事だったのですね。 特に「初心者がやりがちなミス」はほぼ網羅的に地雷踏みまくってます(^^;; http://qiita.com/mpyw/items/b00b72c5c95aac573b71 > あーなるほど、雑誌の記事のハイライトですか。結構重要な機能ですねこれは。 そうなんです。。。。ご理解いただきありがとうございます。 > ここでは分かりやすいように3番目を採用しておきました。 こちらは、よく分かるような気がします。 > 「mysql_connect」「mysql_select_db」「mysql_query」といった関数は何らかの原因でコールに失敗する可能性があり、 構築中のサイトは、私自身がせっせとレコードをINSERTし、 サイト閲覧者はもっぱら雑誌記事目録をSELECTするだけなので あまり例外など発生しないかなーという甘い考えでおりました。。。 ＞「could not find driver」という例外がスローされていることが はい、確かに出ましたが、php.iniの編集で何とかなったようです。 （続きは＃６への御礼にて）
ごめんなさい、HTML生成している部分のコードあんまり読んでいませんでした（汗） >> while ($col = mysql_fetch_array($value)) { ... } 「PHPでデータベースに接続するときのまとめ」を読まれたなら既にお気づきかもしれませんが、Mysql関数は全て非推奨なので、今からでもいいのでPDOクラスを使った方法に乗り換えましょう。Mysql関数は将来のPHPバージョンで削除されることが約束されています。せっかくなのでサンプルコードはこちらの方法で書いてみます。 サンプルコード http://ideone.com/ePy31D データベース接続の部分やSQL文は適宜いじってください。
お礼
To_aru_Userさん、親身に相談にのっていただき、本当にありがとうございます。 PDOの推奨は、半年ほど前に（いつもこちらでお世話になっている）To_aru_Userさんや agunuzさんからもご示唆いただいていたところでした。 http://oshiete.goo.ne.jp/qa/8277847.html 一度プログラムを完成させ、その後でmysql関数のPDO化を図っていこうと考えていましたが、 既存のコードで while ($col = mysql_fetch_array($rst)) { $val = "私の好きな果物は、" . $col['fruit'] . "です"; みたいな、$col['fruit']の部分（連想配列、と呼べばいいのでしょうか） の書き換え方がわからず、PDOへ乗り換えきれていないのが本当のところです。 プログラムの冒頭、↓のようになっているところを、 $con = mysql_connect(test, root, passwotd); $selectdb = mysql_select_db(test, $con); $sql = "select * from mytable"; $rst = mysql_query($sql, $con); $recmax = mysql_fetch_array($rst); echo "私の好きな果物は、" . $col['fruit'] . "です"; PDOにする場合は、↓のように「こちょこちょ」と小手先の変更で済むのでしょうか？？ $pdo = new PDO( 'mysql:dbname=test;host=localhost;charset=utf8', 'root', '', array( PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, ) ); $sql = 'select * from mytable'; $list = $pdo->query($sql)->fetchAll(PDO::FETCH_NUM);
「SQLで取り出してきてHTMLとして表示する準備をした後に特定の部分のみハイライトする」という作業が正しい手順ではないですね…強引に実装するならば functih hightlight_html($query, $html) { // 既にHTML表示する準備がされたあとなのでエスケープしてから検索する $query = htmlspecialchars($query, ENT_QUOTES, 'UTF-8'); // 空白文字を抽出するパターン $regex = "/[\\x0-\x20\x7f\xc2\xa0\xe3\x80\x80]++/u"; // 空白文字で分割して配列化 $keywords = preg_split($regex, htmlspecialchars($query), -1, PREG_SPLIT_NO_EMPTY); // 置換するペアの配列を生成する $pairs = array(); foreach ($keywords as $keyword) { $pairs[$keyword] = "<b>{$keyword}</b>"; } // 置換を実行 return strtr($html, $pairs); } となりますが、属性値中にヒットするキーワードが現れたりするともちろんおかしくなります。実用的なものを作りたいならば根本的に設計からやり直す必要がありそうです、<b></b>タグによるハイライトはあらかじめ行っておいてから残りのHTMLを生成しましょう。単純にSQLで取り出してきた項目に関して、それが指定キーワードの中に存在するかどうかを preg_match 関数や in_array 関数でチェックするだけだと思います。
お礼
To_aru_Userさん、何度もお世話になります。 ＞属性値中にヒットするキーワードが現れたりするともちろんおかしくなります これはどういう状況をさしておりますでしょうか？もしよろしければ、少し詳しくご教示いただけますと幸いです m(_ _)m ＞<b></b>タグによるハイライトはあらかじめ行っておいてから残りのHTMLを生成しましょう。 ご提示の関数hightlight_htmlはそのような手順で組んでいただいていると理解しておりますが、 ＞単純にSQLで取り出してきた項目に関して そういえば、SQL文でSELECTする段階で、$_GET["q"] を <b> $_GET["q"]) </b>に置換する、という方法は（できるとしても）邪道でしょうかね。。。 ----- さて、ご教示のfunctionを本番環境に組み込んでみようと、こんな感じにしてみましたら、 Warning: Missing argument 2 for hightlight_html(), called in D:\www\index.php on line xxx and defined in :\www\index.php on line xx 上記の警告が何十行かでてきました。とんちんかんな組み込み方をしてるのではないかと思いますが．．． <?php functih hightlight_html($query, $html) { $query = htmlspecialchars($query, ENT_QUOTES, 'UTF-8'); $regex = "/[\\x0-\x20\x7f\xc2\xa0\xe3\x80\x80]++/u"; $keywords = preg_split($regex, htmlspecialchars($query), -1, PREG_SPLIT_NO_EMPTY); $pairs = array(); foreach ($keywords as $keyword) { $pairs[$keyword] = "<b>{$keyword}</b>"; } return strtr($html, $pairs); } $body = "<ul>"; while ($col = mysql_fetch_array($value)) { $col["myval1"] = isset($_GET['q']) && is_string($_GET['q']) ? hightlight_html($_GET['q']) : $col["myval1"]; $col["myval2"] = isset($_GET['q']) && is_string($_GET['q']) ? hightlight_html($_GET['q']) : $col["myval2"]; $col["myval3"] = isset($_GET['q']) && is_string($_GET['q']) ? hightlight_html($_GET['q']) : $col["myval3"]; $body .= "<li>好きな食べ物１：" . $col['myval1'] . " 好きな食べ物２：" . $col['myval2'] . " 好きな食べ物３：" . $col['myval3']; } $body .= "</ul>"; ?> <html> <body> <?= $body ?> </body> </html>
ああ、それぞれのキーワードを太文字にしたいってことですか・・・？もしそういうことであれば以下のような感じでどうぞ。 【コード】 function escape_and_highlight($str) { return preg_replace("/[^\\x0-\x20\x7f\xc2\xa0\xe3\x80\x80]++/u", '<b>$0</b>', htmlspecialchars($str, ENT_QUOTES, 'UTF-8')); } $q = isset($_GET['q']) && is_string($_GET['q']) ? escape_and_highlight($_GET['q']) : ''; 【動作確認】 http://ideone.com/fYWIfZ というか正直、全体を <b></b> で囲んだのと大差ないですよね（汗
お礼
To_aru_User、たびたび恐縮です。 ものすごくすっきりしたfunctionにまとめていただき、大変ありがとうございます。 ご推察のように、 <b>あいうえお</b> <b>かきくけこ</b> のように、全体を <b></b> で囲むというわけではなく、 （例１）検索フォームで、 検索キーワード[とうふ ] [検索]←この検索ボタンをクリックすると、 <ul> <li>好きな食べ物１：<b>とうふ</b> 好きな食べ物２：たまごやき 好きな食べ物３：めろん <li>好きな食べ物１：みかん 好きな食べ物２：サラダ 好きな食べ物３：みそ汁 <li>好きな食べ物１：たまご<b>とうふ</b> 好きな食べ物２：たまごやき 好きな食べ物３：チョコ ： ： </ul> 以上のように<b></b>でくくられ、 （例２） 検索キーワード[とうふ たまご ] [検索]←この検索ボタンをクリックすると、 スペース区切りの複数キーワードはorで結ばれるSQLを裏で構築したいため、 <ul> <li>好きな食べ物１：<b>とうふ</b> 好きな食べ物２：<b>たまご</b>やき 好きな食べ物３：めろん <li>好きな食べ物１：みかん 好きな食べ物２：サラダ 好きな食べ物３：みそ汁 <li>好きな食べ物１：<b>たまご</b><b>とうふ</b> 好きな食べ物２：<b>たまご</b>やき 好きな食べ物３：チョコ ： ： </ul> …と以上のような感じにできるといいのですが．．．
【きりがなさ過ぎて要点が分からなくなってるダメ出し】 >> isset($_GET["q"]) and strlen($_GET["q"]) > 0 この書き方をすると「test.php?q[]=str」などの配列のクエリを送信されたときにstrlen関数が E_WARNING レベルのエラーを発生する可能性があります。そもそも1文字以上あるかどうかを調べるためだけにstrlen関数を使うのは冗長ですし、エラーを防ぎたいのであれば文字列であるかどうかも判定して isset($_GET['q']) and is_string($_GET['q']) and $_GET['q'] !== '' とするのが一番確実です。$_POST ならまだしも $_GET はいとも簡単に書き換えが可能なので取り扱いは慎重に。 >> is_null($myval1) $myval1ってどこから出てきたんでしょうか…コードの構成自体が普通ではなさそうです。そして、変数が未定義かどうかをチェックするのはis_null関数ではなくisset命令で行うべきです。is_null関数を使うと未定義のときに E_NOTICE レベルのエラーが発生します。 >> ereg_replace(preg_quote($_GET["q"]), "<b>". "\\0" . "</b>", $myval1); どうしてpreg_quoteはちゃんと使ってるのにpreg_replaceじゃなくてereg_replaceなんでしょうか…PHPには正規表現での文字列置換に使える関数は3種類あります。一通りマニュアルを読んでください。POSIX系正規表現関数は問答無用で非推奨なので使ってはいけません。UTF-8であればPCRE系を使っておけば問題ないです。そもそもこのケースであれば正規表現を使うまでも無く… $myval1 = str_replace($_GET['q'], "<b>{$_GET['q']}</b>", $myval1) これだけで十分に対応できます。が、いずれにせよこれをそのままHTMLとして表示してはいけません。XSS脆弱性が発生します。下記にて紹介しますが、htmlspecialchars関数によるエスケープを忘れないでください。「あらかじめHTMLタグで括っておく」という作業自体があまり望ましいものではなく、通常はPHP変数をHTMLの中に埋め込んで表示するタイミング、例えば <p><?php echo htmlspecialchars($var, ENT_QUOTES, 'UTF-8') ?></p> このときに太字にしたいのであれば <p><b><?php echo htmlspecialchars($var, ENT_QUOTES, 'UTF-8') ?></b></p> とすべきです。毎回こんなことを書くのは面倒なので function h($str) { return htmlspecialchars($str, ENT_QUOTES, 'UTF-8'); } という関数を作っておき、echo短縮構文を利用して <p><b><?=h($var)?></b></p> とするのが一番スマートに書く方法です。これにさっきのstr_replaceの例を適用するのであれば $q = h($_GET['q']); $myval1 = str_replace($q, "<b>{$q}</b>", h($myval1)); といった感じでどうぞ。 >> preg_split('/(\s| )+/', $_GET["q"]) 下記リンク先の「PHPでデータベースに接続するときのまとめ」でも解説していますが、 preg_split("/[\\x0-\x20\x7f\xc2\xa0\xe3\x80\x80]++/u", $_GET['q'], -1, PREG_SPLIT_NO_EMPTY) とした方が確実ですね。 【リンク集】 PHP Manual - ereg_replace(POSIX系) http://www.php.net/manual/ja/function.ereg-replace.php PHP Manual - mb_ereg_replace(鬼車系) http://www.php.net/manual/ja/function.mb-ereg-replace.php PHP Manual - preg_replace(PCRE系) http://www.php.net/manual/ja/function.preg-replace.php PHP Manual - str_replace http://www.php.net/manual/ja/function.str-replace.php PHP Manual - HTMLからの脱出 http://www.php.net/manual/ja/language.basic-syntax.phpmode.php PHP Manual - 制御構造に関する別の構文 http://www.php.net/manual/ja/control-structures.alternative-syntax.php PHP Manual - echo http://www.php.net/manual/ja/function.echo.php PHP Manual - htmlspecialchars http://www.php.net/manual/ja/function.htmlspecialchars.php Qiita - $_GET, $_POST などを受け取る際の処理 http://qiita.com/mpyw/items/2f9955db1c02eeef43ea Qiita - 汎用的な変数構造フィルタリング関数 http://qiita.com/mpyw/items/c39b9ee695a5c2e74627 Qiita - PHPでデータベースに接続するときのまとめ http://qiita.com/mpyw/items/b00b72c5c95aac573b71
お礼
To_aru_Userさん、いつも助けていただいてありがとうございます。 ＞【きりがなさ過ぎて要点が分からなくなってるダメ出し】 とんでもないです、一つ一つ熟読させていただきました。 ＞strlen関数を使うのは冗長 古いphp入門書のサンプルから流用して疑問も感じずそのまま使い続けてましたがそうでしたか… ＞isset($_GET['q']) and is_string($_GET['q']) and $_GET['q'] !== '' 非常に勉強になりますm(_ _)m ＞>> is_null($myval1) ＞ $myval1ってどこから出てきたんでしょうか…コードの構成自体が普通ではなさそうです。 こんな感じで、whileで書き出したリストに一致する文言を太字で目立たせたいという要望です。 （ただ、複数キーワードだと対応できませんでした） <?php $body = "<ul>" while ($col = mysql_fetch_array($value)) { if (isset($_GET["q"]) and strlen($_GET["q"]) > 0) { if (! is_null($col['myval1']) ) { $col['myval1'] = ereg_replace(preg_quote($_GET["q"]), "<b>". "\\0" . "</b>", $col['myval1']); } if (! is_null($col['myval2']) ) { $col['myval2'] = ereg_replace(preg_quote($_GET["q"]), "<b>". "\\0" . "</b>", $col['myval2']); } if (! is_null($col['myval3']) ) { $col['myval3'] = ereg_replace(preg_quote($_GET["q"]), "<b>". "\\0" . "</b>", $col['myval3']); } } $body .= "<li>好きな食べ物１：" . $col['myval1'] . " 好きな食べ物２：" . $col['myval1'] . " 好きな食べ物３：" . $col['myval1']; } $body = "</ul>" ?> <html> <body> <?= $body ?> </body> </html> ＞変数が未定義かどうかをチェックするのはis_null関数ではなくisset命令で行うべき すみません、 if (! is_null($col['myval1']) )は最初いらないと思ったのですが、 $col['myval1']、$col['myval2']、$col['myval3']がNULLのときがあるので なんとなくis_nullを使ってました >> ereg_replace(preg_quote($_GET["q"]), "<b>". "\\0" . "</b>", $myval1); ＞どうしてpreg_quoteはちゃんと使ってるのにpreg_replaceじゃなくてereg_replace すみません、この掲示板で習ってきたこととか、PHP入門書のサンプルなど切り貼りしながら作りこんできたので、きちんとした使い分けを怠ってきてしまいました。マニュアル再確認いたしますm(_ _)m ＞htmlspecialchars関数によるエスケープを忘れないでください。「あらかじめHTMLタグで括っておく」という作業自体があまり望ましいものではなく、 こちらは承知しました。セキュリティに無頓着すぎてお恥ずかしい限りですがあまり望ましいことではないと仰せのニュアンスはなんとなく理解できます。 ＞とするのが一番スマートに書く方法です。 目からうろこすぎて感激してしまいました。。。 >> preg_split('/(\s| )+/', $_GET["q"]) ＞preg_split("/[\\x0-\x20\x7f\xc2\xa0\xe3\x80\x80]++/u", $_GET['q'], -1, PREG_SPLIT_NO_EMPTY) ＞とした方が確実ですね。 私ごときでは永久に思いつかない方法です(^_^; ＞ 【リンク集】 まずは一通り一覧させていただきましたm(_ _)m
お礼
To_aru_Userさん、たびたび本当にありがとうございました。 （本件からしばらく離れていたため、御礼が遅くなって大変失礼いたしました） ご提示いただいたサンプル、大分嚙み砕いて記述いただきましたので おかげさまでわたしでも何とか組み込めそうです。 イメージはよくつかめました、こまごましたテクでつまづいた場合は またお世話になるかもしれませんが、よろしくお願い致します。 ＞実際のDBのデータってどうなってるんでしょうか？ 全て半角に直して格納しているので、ご提示のものでやりくりしてみますm(_ _)m ともかく、本件では大変お世話になりましたこと、御礼申し上げます。 本当にありがとうございました。