めっきり寒い毎日ですが、
きみ、それPHP 4やで
すっかり感傷にふけったところで、
今回は、
診断するのはログイン処理を行うlib/
とcontroller/
の2ファイルです。では、
<?php
require_once('lib/BasicController.php');
require_once('lib/CheckExecuter.php');
require_once('lib/Login.php');
require_once('model/UserModel.php');
define_once('LOGIN_ID', 'login_id');
define_once('PASSWORD', 'password');
/**
* <b>LoginController</b>
* ログインコントローラ
*
* @author hoge
* @version $Id: LoginController.php,v 1.1.1.1 2004/10/01 08:46:04 hoge Exp $
*/
class LoginController extends BasicController {
var $NEXT_PAGE = 'top.php';
/**
* コンストラクタ
*
* @param $values 値連想配列(フォームからの入力の際は、$_POST)
*/
function LoginController() {
$this->init();
// セッションクリア
unset($_SESSION[SESSION_USER]);
if (Http::isPost()) {
$this->values = $_POST;
} else {
$this->values = null;
}
}
/**
* メイン処理
*/
function execute() {
if (Http::isPost()) {
// 値チェック
$checker = new CheckExecuter($this);
$checker->check();
$this->errorMessages = $checker->getErrorMessages();
// 正常なら処理を行う
if (!$this->isError()) {
// セッションクリア
$_SESSION = array();
// ログイン処理
$userModel = new UserModel();
$userModel->setLoginId($this->values[LOGIN_ID]);
$userModel->setPassword($this->values[PASSWORD]);
if (Login::auth($userModel)) {
unset($_SESSION[SESSION_USER]);
$_SESSION[SESSION_USER] = $userModel;
// 次ページへ
$this->transferPage($this->NEXT_PAGE);
} else {
$this->addErrorMessage('error_not_login');
return;
}
}
}
}
/**
* ログインIDチェック
*/
function checkLoginId() {
if (isset($this->values[LOGIN_ID])
&& $this->values[LOGIN_ID]) {
return null;
}
else {
return "check_login_id";
}
}
/**
* ログインIDチェック
*/
function checkLoginId() {
if (isset($this->values[LOGIN_ID])
&& $this->values[LOGIN_ID]) {
return null;
}
else {
return "check_login_id";
}
}
/**
* パスワードチェック
* 半角英数8桁以内
*/
function checkPassword() {
if (isset($this->values[PASSWORD])
&& $this->values[PASSWORD]
&& ereg('^[a-zA-Z0-9]+$', $this->values[PASSWORD])
&& strlen($this->values[PASSWORD]) <= 8) {
return null;
}
else {
return "check_password";
}
}
}
?>
<?php
require_once('lib/Log.php');
require_once('lib/Database.php');
/**
* <b>Login</b>
* ログインクラス
* ログイン管理を行う
*
* @author hoge
* @version $Id: Login.php,v 1.1.1.1 2004/10/01 08:46:04 hoge Exp $
*/
class Login {
/**
* コンストラクタ
*/
function Login() {
}
/**
* 認証
*
* @param &$userModel ユーザ情報Model
* IdCardNoとPasswordをセットしておく
* @return 認証OKならtrue
* エラーならfalse
*/
function auth(&$userModel) {
if (get_class($userModel) != 'usermodel') {
return false;
}
$handleName = addslashes($userModel->getHandleName());
$password = addslashes($userModel->getPassword());
$where = sprintf('handle_name=\'%s\' and password =\'%s\'', $handleName, $password);
$db =& Database::getInstance();
if (($record = $db->getOneRecord('*', 'v_user', $where))) {
// ユーザ情報をセット
$userModel->setId($record['id']);
$userModel->setIdCardNo($record['id_card_no']);
$userModel->setEmail($record['email']);
$userModel->setFirstName($record['first_name']);
$userModel->setLastName($record['last_name']);
$userModel->setFirstNameKana($record['first_kana']);
$userModel->setLastNameKana($record['last_kana']);
$userModel->setPrefecture($record['prefecture']);
$userModel->setAddress($record['address']);
$userModel->setSubAddress($record['sub_address']);
$userModel->setZip($record['zip']);
$userModel->setTel($record['tel']);
$userModel->setBirthdayYear($record['birthday_year']);
$userModel->setBirthdayMonth($record['birthday_month']);
$userModel->setBirthdayDay($record['birthday_day']);
$userModel->setLangCode($record['lang_code']);
return true;
} else {
return false;
}
}
}
?>
診断
1. PHP 4らしいコード
まずコードを見た印象としては、
ざっと見渡しただけでも、require_
文による外部PHPファイルの読み込み、var
キーワードによる定義、static
指定なしのクラスメソッドなどもそうですね。
これらは、
では、
1-1. require_once
文による外部ファイルの読み込み
両コードとも、require_
文で外部のクラスファイルを読み込んでいます。当時、
require_
文は悪くない方法なのですが、include_
を別途指定する必要があるなど、
オートローダは自作することもできますが、
1-2. 名前空間
Composerによるオートローダを導入して、SessionHandler
クラスが導入されて、
名前空間を導入しておくと、User
クラスでも、App
とFooLib
のように別にしておけば、\App\User
、FooLib\User
となり、
namespace App;
class User {}
namespace FooLib;
class User {}
1-3. クラス名と同名のコンストラクタ
コンストラクタのメソッド名がクラス名と同じ名前になっています。
PHP 4のコンストラクタはこのように記述するしかなかったのですが、__
というメソッド名がコンストラクタとして記述できるようになりました。従来の同名コンストラクタも利用できるのですが、__
を使いましょう。
1-4. オブジェクトの参照渡し
PHP 4らしい書き方と言えば、Login
クラスのauth()
メソッドの引数が参照渡しとなっています。
function auth(&$userModel) {
また、
$db =& Database::getInstance();
PHP 4では、
また、Login
クラスのauth()
メソッドの場合、UserModel
クラスのオブジェクトに対してセッターメソッドで値をセットしています。このオブジェクトの変更を呼び出し元でも利用するため、
このようにPHP 5でオブジェクトを代入する際、&
を使った明示的な参照渡しは必要ありません。特にパフォーマンスについては、&
による参照渡しは基本的には使わないほうがよいです。
1-5. メソッドの重複
LoginController
クラスを見てみると、checkLoginId()
というメソッドが2つ定義されています。PHP 5では、
1-6. get_class()関数の挙動の違い
Login
クラスのauth()
メソッドでは、get_
関数でクラス名を取得して、return
するようになっています。
PHP 4では、get_
関数の戻り値はクラス名を小文字にした文字列でした。よって、UserModel
を小文字にしたusermodel
と比較しています。しかし、UserModel
となります。つまり、$userModel
変数がUserModel
クラスのオブジェクトでも、true
になりません。
PHP 5で動かすなら、UserModel
にするか、get_
の戻り値をstrtolower()
関数で小文字に変換する必要があります。
単に引数の型をチェックしたいということであれば、
function auth(UserModel $userModel) {
1-7. ereg()は非推奨
LoginController
クラスのcheckPassword()
メソッドで使われているereg()
関数は、preg_
関数で代用しましょう。
ereg('^[a-zA-Z0-9]+$', $this->values[PASSWORD])
↓
preg_match('/^[a-zA-Z0-9]+$/', $this->values[PASSWORD])
1-8. クラスメソッドにstaticが付いていない
Login
クラスのauth()
メソッドは、LoginController
クラスでの呼び出し方を見るとクラスメソッドを想定しているようです。
if (Login::auth($userModel)) {
このようにクラスメソッドとして呼び出しているメソッドの定義にstatic
キーワードが付いていない場合、
コードを読む際もメソッドがインスタンスメソッドなのかクラスメソッドなのかの判別ができないので、static
キーワードを付けておきしょう。
2. addslashes()によるエスケープ処理
PHP 4固有のことではないのですが、
Login
クラスのauth()
メソッドでは、addslashes()
関数が使われています。addslashes()
関数はマルチバイト文字などを考慮せずに機械的にエスケープを行ってしまうため、
SQLに含める値をエスケープするにはaddslashes()
関数ではなく、
addslashes()
関数を使う場面はおおよそないとも言えるので、
3. 連続したセッターメソッド
これは何かマズイことがあるというわけではないのですが、
Login
クラスのauth()
メソッドでは、$userModel
変数に対してセッターメソッドが15行ほど並んでいます。各セッターメソッドに対して連想配列$record
の対応するキーの値をひたすら渡しています。まあ丁寧でよいのですが、
// 連想配列の値を属性にセットする
$userModel->setAttributes($record);
今回は、