こんにちは。新原です。PHPプログラミング診断室はじまりました。巷に溢れる病めるPHPコードを診断していきたいと思います。
PHPのコードと聞くとどういったイメージを想像されるでしょう? 昔からPHPを知っている方であれば、まずイメージするのが、HTMLとPHPが混在するコードではないでしょうか。HTMLの中にPHPが書けるのは大きなメリットでもあります。ただ、すべての処理がHTMLの中に混在すると、これはなかなか理解しづらいコードになっていきます。もしかすると、そんなコードを見て、PHPに良くないイメージを持った人がいるかもしれません。
初めての診断は、まさにHTMLとPHPが混在するコードです。では、お入りください。
関数定義がなく、流れるようなコードの妙技
今回のPHPコードは、2002年ごろに書かれたものです。とあるWebサイトで稼働していました。内容は、よくあるメールマガジンの申し込みフォームです。フォームでメールアドレスと購読するメールマガジンを入力して送信すると、DB(PostgreSQL)に保存します。元のコードでは、メール送信などの処理もあったのですが、ここでは省略しています。
では、さっそくPHPコードを見てみましょう。
診断
1. ロジックとビューの混在
このコードを見て、まず気になるのが、HTMLの中に多くのPHPコードが含まれている点です。
先頭の<html>
タグからHTMLが開始しているのですが、9行目の<table><tr><td>
タグ以下から、210行目の</td></tr></table>
タグまでの200行がPHPコードになります。このPHPコードでは、状態ごとの処理分岐、入力値のチェック、データベースへのアクセスなど多くの処理を行っています。すべての処理はこのコードの中で実行されています。また、PHPコードの中ではprint
文によるHTMLの出力処理も行われています。
このように、PHPで行う処理(ロジック)とHTMLの出力部分(ビュー)が混在しているコードは、処理の流れが読みづらくなります。特にこのコードは、状態に応じて処理の流れが変化しており、その状態ごとに出力されるHTMLも変わります。このコードを一見しただけで、実際にどのようなHTMLが出力されるかをイメージするのは難しいでしょう。
ビューを動的に出力するためにHTMLの中にPHPコードが混在するのは、問題のない使い方です。ですが、このコードのようにロジックとビューが混在するコードでは、ロジックの流れが追いづらく、出力されるビューも把握しづらいものになります。一般的に、ロジックは状態に応じて処理の流れが変わるので複雑になりがちです。それとビューが混在すると、いつどのようなHTMLが出力されるか、どのような画面になるのかというのが、実際に動かしてみるまでわからないということになります。
理解しづらいコードは、不具合に気づきづらくなり、またその修正や変更も難しいものになります。
まずは、ロジックとビューを明確に分ける。そして、ロジックは先に実行しておいて、ビューではHTML出力に関連する処理のみを書く、というように分離すると役割と流れが明確になり、理解しやすいコードになるでしょう。
2. print文によるHTML出力
PHPコードの中では、HTMLがprint
文にて出力されています。print
文での出力自体が悪いわけではないのですが、HTMLをPHPの文字列として記述しているので、読みづらくなっています。エディタによっては、文字列についてはHTMLと認識せずにコードハイライトなどが効かない場合もあります。HTMLを記述する際も、出力個所をダブルクォーテーションで囲む、HTMLの属性値を囲んでいるダブルクォーテーションをエスケープする、改行コードを入れるなど手間がかかります。
下記が実際にコードの中でprint
文でHTMLを出力している個所の一つです。
このコードを、PHP開始タグ、終了タグを使い、HTML部分をPHPコードから出して書き直したのが以下です。どちらが読みやすいかは一目瞭然でしょう。
HTMLの中でPHPを書く場合、PHP開始タグ、終了タグをうまく組み合わせることで、HTMLタグのように見やすく書けるのがPHPの利点です。ここは素直に、HTMLタグは、PHPの文字列ではなく、HTMLとして記述するようにしましょう。
3. 処理の流れが追いづらい
PHPコード全体の流れを見ていきます。今回のコードは大きく分けて14行目から123行目のswitch
文と、125行目から192行目にあるもう一つのswitch
文に分けられます。どちらも同じ$mode
変数の値で分岐を行っているのですが、なぜかswitch
文が2つに分けられています。実は1つ目のswitch
文の中で$mode
変数の値を書き換えており、この内容によって2つ目のswitch
文による処理を実行するか否かが決定されます。
1つ目のswitch
文では、$mode
変数が空の場合とadd_check
の場合で処理を分岐しています。前者はフォームを表示する初期状態、後者はフォームから値が送信されてきた状態の処理です。add_check
の場合、送信されてきた値のチェックを行い、値が正常であれば$mode
の値をadd
に変更します。そして2つ目のswitch
文では、$mode
変数がadd
の場合のみ登録処理を行う、という流れです。
単純に考えれば、add_check
の処理で入力値検査が正常に終了したあとにそのまま登録処理を続ければよいと思うのですが、なぜかswitch
文を分けるという構造になっています。なぜこうなったかの理由はわかりませんが、あとでコードを追う身としてはわかりにくいことこのうえないです。
この2つのswitch
文による条件分岐は、単にわかりにくいだけではありません。$mode
変数の値はフォームから送信されてくることが想定されているので、任意に変更することが可能です。つまり、もし$mode
の値をadd
に改ざんして送信すると、1つ目のswitch
文を飛ばして、いきなり2つ目のswitch
文にて登録処理を実行できてしまいます。こうなると、せっかくの入力値検査をスキップしてしまうので、任意の値を登録されてしまう可能性があります。これは良くないですね。
改善策としては、まずは全体の流れがわかりやすいように素直にコードを書きましょうということです。このコードで必要な状態は「フォームを表示する(初期状態)」と「フォームから値が送信される(送信された値を処理する状態)」の2つしかないわけです。そうであれば、まずは全体をその2つに分ける。そしてそれぞれの処理を行うコードを書くというのが見通しも良く、理解しやすいコードになります。
4. 外部から送信されてきた値の参照
前項の$mode
変数の値は、外部から送信されると書きました。このコードを見て、なぜそう動くのかがわかる人はPHP歴がなかなか長い方です。このコードでは、$_GET
や$_POST
、$_REQUEST
などHTTPリクエストで送られてくる値を参照するスーパーグローバル変数が見当たりません。では、どうやって$mode
変数に送信されてきた値が入るのでしょうか。実は、以前のPHPにはregister_globals
という機能があり、これを有効にすると外部から送信されてきた値をPHPの変数値として参照することが可能となっていました。
つまり、クエリストリングやフォームから送信された値にmode
というパラメータがあると、$mode
変数にその値が格納されるという具合です。
一見便利なようにも見えるのですが、システム外部から任意の値をセットできてしまうので、セキュリティ上問題になることが多くありました。そこで、スーパーグローバル変数の登場以後は、この機能は非推奨となり、PHP 5.4では完全に廃止されました。
また、外部から送信される値は、いかようにも改ざんすることが可能です。外部から送信される値を元に処理を分岐させること自体は問題ないのですが、その場合は、どのような値が送信されてきても動作に支障がないように注意する必要があります。
5. 同じ処理を実直に書き続けている
このコードは実に実直です。まじめにこつこつと書かれています。まったく同じコードが何度も何度も丁寧に書かれています。同じことを同じように寸分違わず、こつこつと積み重ねていく。人生訓として大切に思える言葉ですが、コードにおいてはこういった誠実さは不要です。できるだけ、ずぼらになって同じコードは書かない、繰り返さないということが必要です。
たとえば、このコードではpg_connect()
関数が何度も書かれています。しかもDBに接続するための情報も引数に何度も書かれています。同じ作業に嫌気をささずに、実直にしたためられているのがわかります。ただ、これはコードを変更する際に大きな足枷となります。もしDBの接続先が変わったら、あちらこちらのコードを直す必要があります。困りましたね。あと、何より書くのが面倒です。同じ処理を何度も書くのは本当に無駄です。面倒、無駄と書くと怠惰なように思えるかもしれませんが、むしろコードを書くうえでの怠惰は奨励されるべきことです。
では、どうすればよいか。簡単です。同じ処理は関数にまとめてしまえばよいのです。
pg_connect()
関数であれば、下記のようにconnect_db()
などの関数を定義して、実際にDBに接続する際はこの関数を呼びます。こうすれば、いざ接続先を変更したいとなってもconnect_db()
だけを修正すればよいのです。
さらに接続情報などの設定値は環境に応じて変わるものなので、コードの中に直接書くのではなく、コード上部などに定数などで定義しておきます。こうすれば設定を変更したいときは、コードを読まなくても設定値だけを変更すればよくなります。
前項でロジックとビューを分離したように、内容や変更頻度に応じて、コードを分離すると、変更しやすく、理解しやすいコードになります。
治療すべきポイントを洗い出したところで、次回はいよいよこれらの問題点を治療していきます。