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


Ignore:
Timestamp:
2011/03/23 16:34:27 (15 years ago)
Author:
nanasess
Comment:

--

Legend:

Unmodified
Added
Removed
Modified

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

    v7 v8  
    1 = リファクタリングガイドライン = 
    2  
    3 == init 関数 == 
    4 init 関数は, クラスの初期化を目的する. 
    5 ビジネスロジックの記述はしないこと 
    6  
    7 {{{ 
    8 #!php 
    9 <?php 
    10 function init() { 
    11  
    12     /** 
    13      * NG ビジネスロジックの記述はしない 
    14      */ 
    15     if (count($this->arrPayment) > 0) { 
    16         $i = 0; 
    17         foreach ($this->arrPayment as $val) { 
    18             $this->payment[$i] = $val; 
    19             $i++; 
    20         } 
    21     } 
    22  
    23     /** 
    24      * OK クラスの初期化のみ行う 
    25      */ 
    26     $this->tpl_mainpage = 'index.tpl'; 
    27     $this->arrDISP = $masterData->getMasterData('mtb_disp'); 
    28 } 
    29 ?> 
    30 }}} 
    31  
    32 == action 関数 == 
    33  
    34 action 関数は MVC のコントローラにあたり以下の処理を記述する. 
    35  
    36  * 条件分岐 
    37  * VIEW からの入力 
    38  * VIEW に渡すメンバ変数($this->variable_name)への代入 
    39  * 宣言 
    40  * ビジネスロジックの呼び出し 
    41  
    42 action 関数に, ビジネスロジックを直接記述したり, SQL を実行する処理を記述しないこと. 
    43  
    44 {{{ 
    45 #!php 
    46 <?php 
    47 /** 
    48  * action 関数のサンプル 
    49  */ 
    50 function action() { 
    51  
    52     /** OK VIEW からの入力を処理する */ 
    53     $this->arrForm = $this->lfConvertParam($_POST); 
    54  
    55     /** OK 入力に応じて条件分岐する */ 
    56     switch ($this->getMode()) { 
    57     case 'edit': 
    58  
    59         /** OK エラーの内容に応じて条件分岐する */ 
    60         $this->arrErr = $this->lfProductClassError($this->arrForm); 
    61         if (empty($this->arrErr)){ 
    62             $this->tpl_mainpage = 'products/product_class_confirm.tpl'; 
    63             $this->lfProductConfirmPage($this->arrForm); 
    64         } else { 
    65             $this->doPreEdit($this->arrForm, false ,true); 
    66         } 
    67         break; 
    68  
    69     case 'delete': 
    70         $this->doDelete($this->arrForm); 
    71         break; 
    72  
    73     case 'disp': 
    74         /** NG ビジネスロジックの記述をしてはならない */ 
    75         foreach ($this->arrForm as $key => $val) { 
    76             $foo[$key] = $val; 
    77         } 
    78  
    79         /** NG SQLを実行する処理を記述してはならない */ 
    80         $objQuery =& SC_Query::getSingletonInstance(); 
    81         $arrResults = $objQuery->select('*', 'table_name'); 
    82  
    83         $this->doDisp($this->arrForm); 
    84         break; 
    85  
    86     case 'complete': 
    87         $this->registerProductClass($this->arrForm, $this->arrForm['product_id']); 
    88         SC_Response_Ex::sendRedirect('complete.php'); 
    89         break; 
    90  
    91     default: 
    92     } 
    93 } 
    94 ?> 
    95 }}} 
    96  
    97 == MODE パラメータ == 
    98  
    99 `$_POST['mode']` や `$_GET['mode']` に対する条件分岐は, LC_Page::getMode() を使用し, switch 文で記述する. 
    100  
    101 {{{ 
    102 #!php 
    103 <?php 
    104 /** OK switch 文で mode の分岐を行う */ 
    105 switch ($this->getMode()) { 
    106 case 'disp': 
    107     $this->doDisp($this->arrForm); 
    108     break; 
    109  
    110 case 'complete': 
    111     SC_Response_Ex::sendRedirect('complete.php'); 
    112     break; 
    113 default: 
    114 } 
    115  
    116 /** NG mode の条件分岐に if 文を使用してはならない */ 
    117 if ($_POST['mode'] == 'disp') { 
    118     $this->doDisp($this->arrForm); 
    119 } elseif ($_POST['mode'] == 'complete') { 
    120     SC_Response_Ex::sendRedirect('complete.php'); 
    121 } 
    122 ?> 
    123 }}} 
    124  
    125 == ビジネスロジック == 
    126  
    127   * ページ間で重複する処理が存在する場合は, Helper などの共通クラスへ記述する 
    128   * ページ固有の処理は, ローカル関数で記述する 
    129   * 可能であれば, PHPUnit を使用してテストケースを残すこと 
    130   * 宣言を除き, 引数や返り値が無く, すべて内部のメンバ変数で処理するような関数は極力作成しない 
    131     * ステートレスな処理を心掛けること 
    132  
    133 {{{ 
    134 #!php 
    135 <?php 
    136  
    137 /** 
    138  * NG 引数ではなく, クラスのメンバ変数を使用して振舞いを変更する 
    139  */ 
    140 function lfSendMail() { 
    141      $objPurchase = new SC_Purchase_Ex(); 
    142      $objPurchase->sendOrderMail($this->order_id) 
    143 } 
    144  
    145 /** 
    146  * OK 引数で振舞いを変更できる 
    147  */ 
    148 function lfSendMail($order_id) { 
    149      $objPurchase = new SC_Purchase_Ex(); 
    150      $objPurchase->sendOrderMail($order_id) 
    151 } 
    152  
    153 /** 
    154  * NG 引数や返り値が無く, メンバ変数の値を直接変更している 
    155  */ 
    156 function lfConvertLoginPass(){ 
    157     if(strlen($this->arrForm['login_pass']) < 1 ) { 
    158         return; 
    159     } 
    160     $this->arrForm['login_pass'] = trim($this->arrForm['login_pass']); 
    161     $this->arrForm['login_pass1'] = $this->arrForm['login_pass']; 
    162     $this->arrForm['login_pass2'] = $this->arrForm['login_pass']; 
    163 } 
    164  
    165 /** 
    166  * OK ローカル関数内では, ローカル変数を使用することで, 拡張性, 保守性が向上し, ユニットテストも書きやすい 
    167  */ 
    168  
    169 $this->arrForm = $this->lfConvertLoginPass($this->arrForm); 
    170  
    171 function lfConvertLoginPass(&$arrForm){ 
    172     if(strlen($arrForm['login_pass']) < 1 ) { 
    173         return; 
    174     } 
    175     $arrForm['login_pass'] = trim($arrForm['login_pass']); 
    176     $arrForm['login_pass1'] = $arrForm['login_pass']; 
    177     $arrForm['login_pass2'] = $arrForm['login_pass']; 
    178     return $arrForm; 
    179 } 
    180  
    181 /** 
    182  * ユニットテストの例. 
    183  */ 
    184 function testLfConvertLoginPass() { 
    185     $login_pass = 'login_pass_value'; 
    186     $arrForm = array('login_pass' => $login_pass); 
    187  
    188     $expected = array('login_pass' => $login_pass, 
    189                       'login_pass1' => $login_pass, 
    190                       'login_pass2' => $login_pass); 
    191  
    192     $actual = lfConvertLoginPass($arrForm); 
    193  
    194     $this->assertEquals($expected, $actual); 
    195 } 
    196 ?> 
    197 }}} 
    198  
    199  * ループ処理などで, メンバ変数や, スーパーグローバル変数は直接使わない 
    200  
    201 {{{ 
    202 #!php 
    203 <?php 
    204  
    205 /** 
    206  * NG ループ処理で, メンバ変数やスーパーグローバル変数を直接扱っている 
    207  */ 
    208 foreach ($_POST['params'] as $key => $val) { 
    209      $this->arrForm[$key] = $val; 
    210 } 
    211  
    212 /** 
    213  * OK メンバ変数や, スーパーグローバル変数に作用するループ処理は, 別途関数を作成する 
    214  */ 
    215 $this->arrParams = $this->getParams($_POST['params']); 
    216  
    217 function getParams($arrParams) { 
    218      $arrResults = array(); 
    219      foreach ($arrParams as $key => $val) { 
    220          $arrResults[$key] = $val; 
    221      } 
    222      return $arrResults; 
    223 } 
    224 ?> 
    225 }}} 
    226  
    227 == データベースアクセス == 
    228  
    229 特に理由の無い場合は `SC_Query::getSingletonInstance()` を使用してインスタンスを取得すること 
    230  
    231 {{{ 
    232 #!php 
    233 <?php 
    234  
    235 /** 
    236  * NG PHP4 環境では, 新たなインスタンスが生成されてしまう 
    237  */ 
    238 $objQuery = new SC_Query(); 
    239  
    240 /** 
    241  * OK シングルトンが保証され, トランザクションの扱いが容易になる 
    242  */ 
    243 $objQuery =& SC_Query::getSingletonInstance(); 
    244  
    245 ?> 
    246 }}} 
    247  
    248 SQL 文を散乱させない. 
    249 SELECT id, name, foo, bar FROM table_name と SELECT id, name, foo, bar, too FROM table_name が存在する場合は, 後者を共通関数にリファクタリングすること 
    250  
    251 {{{ 
    252 #!php 
    253 <?php 
    254  
    255 /** 
    256  * NG 類似した SQL を散乱させてはいけない 
    257  */ 
    258 $arrResults = $objQuery->select('id, name, foo, bar', 'table_name'); 
    259 $arrResult2 = $objQuery->select('id, name, foo, bar, too', 'table_name', 'too = ?', $arrParams['too']); 
    260  
    261 /** 
    262  * OK カラム名や, WHERE の違いなどは共通関数で吸収する 
    263  */ 
    264 function getResults($arrParams = array()) { 
    265     $where = ''; 
    266     $arrValues = array(); 
    267     if (isset($arrParams['too'])) { 
    268         $where .= 'too = ?'; 
    269         $arrValues[] = $arrParams['too']; 
    270  
    271     } 
    272     $objQuery =& SC_Query::getSingletonInstance(); 
    273     $arrResults = $objQuery->select('*', 'table_name', $where, $arrValues); 
    274     return $arrResults; 
    275 } 
    276 $arrResults = getResults(); 
    277 $arrResults2 = getResults($arrParams); 
    278  
    279 ?> 
    280 }}} 
    281  
    282 INSERT/UPDATE/DELETE は, SC_Query::insert(), SC_Query::update(),  SC_Query::delete() を使用し, SC_Query::query() は使用しないこと 
    283  
    284 {{{ 
    285 #!php 
    286 <?php 
    287  
    288 /** NG SC_Query::query() は使用しない */ 
    289 $objQuery =& SC_Query::getSingletonInstance(); 
    290 $objQuery->query('INSERT INTO table_name (col1, col2) VALUES (?, ?)', arrary($col1, $col2)); 
    291  
    292 /** OK SC_Query::insert(), SC_Query::update(),  SC_Query::delete() を使用する */ 
    293 $objQuery =& SC_Query::getSingletonInstance(); 
    294 $objQuery->insert('table_name', array('col1' => $col1, 
    295                                       'col2' => $col2)); 
    296  
    297 ?> 
    298 }}} 
    299 == 入力チェック == 
    300  
    301 入力チェック用のクラスには, SC_FromParam と SC_CheckError が存在するが, 極力 SC_FormParam を使用すること 
    302  
    303 == 端末種別の判別 == 
    304  
    305 `Net_UserAgent_Mobile::isMobile()` や `MOBILE_SITE` 定数など存在するが, `SC_Display::detectDevice()` に統一する 
    306  
    307 == スーパーグローバル変数 == 
    308  
    309 `$_POST` や `$_GET` の値は, 必ず入力チェック後に使用する. `SC_FormParam` を使用していれば, `SC_FormParam::getHashArray()` で取得できる 
    310  
    311 {{{ 
    312 #!php 
    313 <?php 
    314  
    315 /** NG $_POST の値を入力チェックせずに使用する */ 
    316 $this->register($_POST); 
    317  
    318 /** OK 必ず入力チェックを行う */ 
    319 $objFormParam = new SC_FormParam(); 
    320 $objFormParam->addParam('商品規格ID', 'product_class_id', INT_LEN, 'n', array('EXIST_CHECK', 'MAX_LENGTH_CHECK', 'NUM_CHECK')); 
    321 $objFormParam->setParam($_POST); 
    322 $objFormParam->convParam(); 
    323 $arrErr = $objFormParam->checkError() 
    324 if (SC_Utils_Ex::isBlank($arrErr)) { 
    325      // 入力チェック後の値が取得可能 
    326      $arrForm = $objFormParam->getHashArray(); 
    327 } 
    328 $this->register($arrForm); 
    329  
    330 ?> 
    331 }}} 
    332  
    333 `$_SESSION` は, `SC_CartSession クラス`等, セッションを扱うビジネスロジックを通じて使用するのが望ましい. 
    334  
    335 どうしても, グローバル変数を使用したい場合は, global キーワードは使用せず, `$GLOBALS` を使用すること. 
    336  
    337 {{{ 
    338 #!php 
    339 <?php 
    340 /** 
    341  * NG global キーワードの使用すると, 変数名衝突の原因となる 
    342  */ 
    343 function globalSample() { 
    344     global $conn; 
    345  
    346     // 外部からは $conn という変数を使用しているのがわからない 
    347     if (is_null($conn)) { 
    348         $conn =& new ClassName(); 
    349     } 
    350 } 
    351  
    352 /** 
    353  * OK $GLOBALS を使用すると, ローカル変数との衝突の心配は無い 
    354  */ 
    355 function globalSample() { 
    356     if (is_null($GLOBALS['_conn'])) { 
    357         $GLOBALS['_conn'] = new ClassName(); 
    358     } 
    359 } 
    360 ?> 
    361 }}} 
    362  
    363 == 変数名 == 
    364  
    365 変数名で, どんなデータか識別できるよう考慮すること 
    366  
    367 {{{ 
    368 #!php 
    369 <?php 
    370  
    371 /** 
    372  * NG 一見して, どんなデータが格納されているかわからない 
    373  */ 
    374 $arrRet = getProducts(); 
    375  
    376 /** 
    377  * OK 商品データの配列だと, 変数名で判別可能 
    378  */ 
    379 $arrProducts = getProducts(); 
    380  
    381 ?> 
    382 }}} 
    383  
    384 リテラルを格納する (つまり配列・オブジェクト以外の) 変数名は, すべて小文字を使用し, アンダーバーで区切ること 
    385  
    386 {{{ 
    387 #!php 
    388 <?php 
    389  
    390 /** 
    391  * リテラルを格納する変数名は, 連想配列の添字で使用されることも考慮し 
    392  * アンダーバー区切りの方が扱いやすい 
    393  */ 
    394  
    395 /** 
    396  * NG 
    397  */ 
    398 $productId = getProductId(); 
    399  
    400 /** 
    401  * OK 
    402  */ 
    403 $product_id = getProductId(); 
    404  
    405 ?> 
    406 }}} 
    407  
    408 == 1行の文字数/行数 == 
    409  * 1行の文字数は80文字までにすることを目指すこと. すなわち, コードの長さを現実的な範囲で80文字までにおさめるよう努力すべきです. しかしながら, 場合によっては少々長くなってしまってもかまいません. 
    410  * 1関数の行数は多くても200行程度に留めるのが望ましい. これ以上行数が増えてしまう場合は, 関数を分割する等, コーディングの見直しを行ってください 
    411  
    412 == PHPDoc コメント == 
    413  
    414 関数の PHPDoc コメントは必ず記述する. 
    415   * 関数の説明 
    416   * 引数 
    417     * 引数の無い関数は省略可能 
    418   * 返り値 
    419     * 返り値が無くても void を明記すること 
    420  
    421 == 非推奨機能 == 
    422  
    423 error_reporting(E_ALL) で, エラー及び警告が出力されないようコーディングすること. 
    424  
    425 PHP5.3 で非推奨となっている関数は使用しないこと. 
    426  
    427   * call_user_method() (かわりに call_user_func() を使用します) 
    428   * call_user_method_array() (かわりに call_user_func_array() を使用します) 
    429   * define_syslog_variables() 
    430   * dl() 
    431   * ereg() (かわりに preg_match() を使用します) 
    432   * ereg_replace() (かわりに preg_replace() を使用します) 
    433   * eregi() (かわりに preg_match() で 'i' 修正子を使用します) 
    434   * eregi_replace() (かわりに preg_replace() で 'i' 修正子を使用します) 
    435   * set_magic_quotes_runtime() およびそのエイリアスである magic_quotes_runtime() 
    436   * session_register() (かわりにスーパーグローバル `$_SESSION` を使用します) 
    437   * session_unregister() (かわりにスーパーグローバル `$_SESSION` を使用します) 
    438   * session_is_registered() (かわりにスーパーグローバル `$_SESSION` を使用します) 
    439   * set_socket_blocking() (かわりに stream_set_blocking() を使用します) 
    440   * split() (かわりに preg_split() を使用します) 
    441   * spliti() (かわりに preg_split() で 'i' 修正子を使用します) 
    442   * sql_regcase() 
    443   * is_dst を mktime() に渡すこと。 かわりにタイムゾーン処理用の新しい関数を使用します。 
    444  
    445 以下の機能も, PHP5.3.x で非推奨となっているが, PHP4互換のために使用しても良い 
    446  
    447   * new の返り値を参照で代入すること 
    448   * 呼び出し時の参照渡し 
    449  
    450 参考 
    451 http://www.php.net/manual/ja/migration53.deprecated.php 
    452  
    453 == PHP4 での動作 == 
    454  
    455 PHP4.3.11 以降で動作可能な構文にすること 
    456  
    457 参考 
    458 http://svn.ec-cube.net/open_trac/ticket/854 
    459 http://svn.ec-cube.net/open_trac/changeset/20024 
    460  
    461 == 不要な機能 == 
    462  
    463 #793 に該当する機能は削除すること 
    464 http://svn.ec-cube.net/open_trac/ticket/793 
     1このページは [wiki:EC-CUBE標準規約/リファクタリングガイドライン] へ移動しました