Changes between Initial Version and Version 1 of リファクタリングガイドライン

Timestamp:

2011/01/27 21:48:19 (15 years ago)

Author:

nanasess

Comment:

--

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

@@ +1 @@
+= リファクタリングガイドライン =
+
+''このページは書きかけです''
+
+== init 関数 ==
+
+変数の初期化のみを行い, ビジネスロジックは記述しないこと
+
+{{{
+#!php
+<?php
+function init() {
+     // 変数の初期化は OK
+     $this->tpl_mainpage = 'index.tpl';
+     $this->arrDISP = $masterData->getMasterData("mtb_disp");
+
+     // ビジネスロジックの記述は NG
+     if (count($this->arrPayment) > 0) {
+         $i = 0;
+         foreach ($this->arrPayment as $val) {
+             $this->payment[$i] = $val;
+             $i++;
+         }
+     }
+}
+?>
+}}}
+== action 関数 ==
+
+action 関数は, 以下の処理のみを記述し, ビジネスロジックは記述しない
+  * 条件分岐
+  * VIEW に渡すメンバ変数($this->variable_name)への代入
+  * 宣言
+  * ビジネスロジックの呼び出し
+
+また, action 関数の内部で, 直接 SQL を実行する処理を記述してはならな 
+い.
+
+{{{
+#!php
+<?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
+<?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
+<?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
+<?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
+<?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` を使用すること.
+
+== 変数名 ==
+
+変数名で, どんなデータか識別できるよう考慮すること
+
+{{{
+誤)
+$arrRet
+
+正)
+$arrProducts
+}}}
+
+テーブルカラム名と共通の変数名は, キャメル式を使用せず, アンダーバーで区切ること
+
+{{{
+誤)
+$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
+