wiki:リファクタリングガイドライン

Version 2 (modified by nanasess, 13 years ago) (diff)

--

リファクタリングガイドライン

このページは書きかけです

init 関数

init 関数は, クラスの初期化を目的とします. ビジネスロジックの記述をしてはいけません.

<?php
function init() {

    /**
     * NG ビジネスロジックの記述はしない
     */
    if (count($this->arrPayment) > 0) {
        $i = 0;
        foreach ($this->arrPayment as $val) {
            $this->payment[$i] = $val;
            $i++;
        }
    }

    /**
     * OK クラスの初期化のみ行う
     */
    $this->tpl_mainpage = 'index.tpl';
    $this->arrDISP = $masterData->getMasterData("mtb_disp");
}

action 関数

action 関数は, 以下の処理のみを記述し, ビジネスロジックは記述しない

  • 条件分岐
  • VIEW に渡すメンバ変数($this->variable_name)への代入
  • 宣言
  • ビジネスロジックの呼び出し

また, action 関数の内部で, 直接 SQL を実行する処理を記述してはならな い.

<?php
function action() {

         $this->arrForm = $this->lfConvertParam($_POST);

         switch ($_POST['mode']) {
         case 'edit':

             $this->arrErr = $this->lfProductClassError($this->arrFor 
m);
             if (empty($this->arrErr)){
                 $this->tpl_mainpage = 'products/product_class_confir 
m.tpl';
                 // 確認ページ表示
                 $this->lfProductConfirmPage($this->arrForm);
             } else {
                 $this->doPreEdit($this->arrForm, false ,true);
             }
             break;

         case 'delete':
             $this->doDelete($this->arrForm);
             break;

         case 'pre_edit':
             $this->doPreEdit($this->arrForm);
             break;

         case 'disp':
             $this->doDisp($this->arrForm);
             break;

         case 'complete':
             // 登録処理
             $this->registerProductClass($this->arrForm, $this->arrFo 
rm['product_id']);
             // 完了ページへリダイレクト
             SC_Response_Ex::sendRedirect("complete.php");
             break;

         default:
         }

}
?>

MODE パラメータ

$_POST['mode']$_GET['mode'] に対する条件分岐は, LC_Page::getMode() を使用し, switch 文で記述する.

<?php
// OK
switch ($this->getMode()) {
     case 'disp':
         $this->doDisp($this->arrForm);
         break;

     case 'complete':
         SC_Response_Ex::sendRedirect("complete.php");
         break;

     default:
     }
}

// NG
if ($_POST['mode'] == 'disp') {
     $this->doDisp($this->arrForm);
} elseif ($_POST['mode'] == 'complete') {
     SC_Response_Ex::sendRedirect("complete.php");
}
?>

ビジネスロジック

  • ページ間で重複する処理が存在する場合は, Helper などの共通クラスへ 記述する
  • ページ固有の処理は, ローカル関数で記述する
  • 宣言を除き, 引数や返り値が無く, すべて内部のメンバ変数で処理するような関数は極力作成しない
    • ステートレスな処理を心掛けること
<?php
// OK
function lfSendMail($order_id) {
     $objPurchase = new SC_Purchase_Ex();
     $objPurchase->sendOrderMail($order_id)
}

// NG
function lfSendMail() {
     $objPurchase = new SC_Purchase_Ex();
     $objPurchase->sendOrderMail($this->order_id)
}
?>
  • 可能であれば, PHPUnit を使用してテストケースを残すこと
  • ループ処理などで, メンバ変数や, スーパーグローバル変数は直接使わない
<?php
// OK
$this->arrParams = $this->getParams($_POST['params']);

function getParams($arrParams) {
     $arrResults = array();
     foreach ($arrParams as $key => $val) {
         $arrResults[$key] = $val;
     }
     return $arrResults;
}

// NG
foreach ($_POST['params'] as $key => $val) {
     $this->arrForm[$key] = $val;
}
?>

データベースアクセス

  • SC_Query のインスタンスを毎回必ず生成する必要の無い場合は SC_Query::getSingletonInstance() を使用してインスタンスを取得すること
  • SQL 文を散乱させない
    • SELECT id, name, foo, bar FROM table_name と SELECT id, name, foo, bar, too FROM table_name が存在する場合は, 後者を共通関数にリファクタリングすること
  • INSERT/UPDATE/DELETE は, SC_Query::insert(), SC_Query::update(), SC_Query::delete() を使用し, SC_Query::query() は使用しないこと

入力チェック

入力チェック用のクラスには, SC_FromParam と SC_CheckError が存在するが, 極力 SC_FormParam を使用すること

スーパーグローバル変数

$_POST や $_GET の値は, 必ず入力チェック後に使用する. SC_FormParam を使用していれば, SC_FormParam::getHashArray() で取得できる

<?php
$objFormParam = new SC_FormParam();
$objFormParam->addParam("商品規格ID", "product_class_id", INT_LEN, "n 
", array("EXIST_CHECK", "MAX_LENGTH_CHECK", "NUM_CHECK"));
$objFormParam->setParam($_POST);
$objFormParam->convParam();
$arrErr = $objFormParam->checkError()
if (SC_Utils_Ex::isBlank($arrErr)) {
     // 入力チェック後の値が取得可能
     $arrForm = $objFormParam->getHashArray();
}
?>

$_SESSION は, SC_CartSession クラス等, セッションを扱うビジネスロジックを通じて使用すること.

どうしても, グローバル変数を使用したい場合は, global キーワードは使用せず, $GLOBALS を使用すること.

変数名

変数名で, どんなデータか識別できるよう考慮すること

<?php

/** 違反サンプル */
$arrRet = getProducts();

/** 修正サンプル */
$arrProducts = getProducts();

テーブルカラム名と共通の変数名は, キャメル式を使用せず, アンダーバーで区切ること

誤)
$productId

正)
$product_id

PHPDoc コメント

関数の PHPDoc コメントは必ず記述する.

  • 関数の説明
  • 引数
  • 返り値

非推奨機能

error_reporting(E_ALL) で, エラー及び警告が出力されないようコーディン グすること. PHP5.3 で非推奨となっている関数は使用しないこと.

  • call_user_method() (かわりに call_user_func() を使用します)
  • call_user_method_array() (かわりに call_user_func_array() を使用します)
  • define_syslog_variables()
  • dl()
  • ereg() (かわりに preg_match() を使用します)
  • ereg_replace() (かわりに preg_replace() を使用します)
  • eregi() (かわりに preg_match() で 'i' 修正子を使用します)
  • eregi_replace() (かわりに preg_replace() で 'i' 修正子を使用します)
  • set_magic_quotes_runtime() およびそのエイリアスである magic_quotes_runtime()
  • session_register() (かわりにスーパーグローバル $_SESSION を使用します)
  • session_unregister() (かわりにスーパーグローバル $_SESSION を使用します)
  • session_is_registered() (かわりにスーパーグローバル $_SESSION を使用します)
  • set_socket_blocking() (かわりに stream_set_blocking() を使用します)
  • split() (かわりに preg_split() を使用します)
  • spliti() (かわりに preg_split() で 'i' 修正子を使用します)
  • sql_regcase()
  • is_dst を mktime() に渡すこと。 かわりにタイムゾーン処理用の新しい関数を使用します。

以下の機能も, PHP5.3.x で非推奨となっているが, PHP4互換のために使用しても良い

  • new の返り値を参照で代入すること
  • 呼び出し時の参照渡し

参考  http://www.php.net/manual/ja/migration53.deprecated.php

PHP4 での動作

PHP4.3.11 以降で動作可能な構文にすること

参考 http://svn.ec-cube.net/open_trac/ticket/854 http://svn.ec-cube.net/open_trac/changeset/20024

不要な機能

TODO