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


Ignore:
Timestamp:
2011/01/28 12:26:43 (15 years ago)
Author:
nanasess
Comment:

体裁修正

Legend:

Unmodified
Added
Removed
Modified

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

    v2 v3  
    11= リファクタリングガイドライン = 
    22 
    3 ''このページは書きかけです'' 
    4  
    53== init 関数 == 
    6 init 関数は, クラスの初期化を目的とします. 
    7 ビジネスロジックの記述をしてはいけません. 
     4init 関数は, クラスの初期化を目的する. 
     5ビジネスロジックの記述はしないこと 
    86 
    97{{{ 
    … …  
    3331== action 関数 == 
    3432 
    35 action 関数は, 以下の処理のみを記述し, ビジネスロジックは記述しない 
    36   * 条件分岐 
    37   * VIEW に渡すメンバ変数($this->variable_name)への代入 
    38   * 宣言 
    39   * ビジネスロジックの呼び出し 
    40  
    41 また, action 関数の内部で, 直接 SQL を実行する処理を記述してはならな  
    42 い. 
    43  
    44 {{{ 
    45 #!php 
    46 <?php 
     33action 関数は MVC のコントローラにあたり以下の処理を記述する. 
     34 
     35 * 条件分岐 
     36 * VIEW からの入力 
     37 * VIEW に渡すメンバ変数($this->variable_name)への代入 
     38 * 宣言 
     39 * ビジネスロジックの呼び出し 
     40 
     41action 関数に, ビジネスロジックを直接記述したり, SQL を実行する処理を記述しないこと. 
     42 
     43{{{ 
     44#!php 
     45<?php 
     46/** 
     47 * action 関数のサンプル 
     48 */ 
    4749function action() { 
    4850 
    49          $this->arrForm = $this->lfConvertParam($_POST); 
    50  
    51          switch ($_POST['mode']) { 
    52          case 'edit': 
    53  
    54              $this->arrErr = $this->lfProductClassError($this->arrFor  
    55 m); 
    56              if (empty($this->arrErr)){ 
    57                  $this->tpl_mainpage = 'products/product_class_confir  
    58 m.tpl'; 
    59                  // 確認ページ表示 
    60                  $this->lfProductConfirmPage($this->arrForm); 
    61              } else { 
    62                  $this->doPreEdit($this->arrForm, false ,true); 
    63              } 
    64              break; 
    65  
    66          case 'delete': 
    67              $this->doDelete($this->arrForm); 
    68              break; 
    69  
    70          case 'pre_edit': 
    71              $this->doPreEdit($this->arrForm); 
    72              break; 
    73  
    74          case 'disp': 
    75              $this->doDisp($this->arrForm); 
    76              break; 
    77  
    78          case 'complete': 
    79              // 登録処理 
    80              $this->registerProductClass($this->arrForm, $this->arrFo  
    81 rm['product_id']); 
    82              // 完了ページへリダイレクト 
    83              SC_Response_Ex::sendRedirect("complete.php"); 
    84              break; 
    85  
    86          default: 
    87          } 
    88  
     51    /** OK VIEW からの入力を処理する */ 
     52    $this->arrForm = $this->lfConvertParam($_POST); 
     53 
     54    /** OK 入力に応じて条件分岐する */ 
     55    switch ($this->getMode()) { 
     56    case 'edit': 
     57 
     58        /** OK エラーの内容に応じて条件分岐する */ 
     59        $this->arrErr = $this->lfProductClassError($this->arrForm); 
     60        if (empty($this->arrErr)){ 
     61            $this->tpl_mainpage = 'products/product_class_confirm.tpl'; 
     62            $this->lfProductConfirmPage($this->arrForm); 
     63        } else { 
     64            $this->doPreEdit($this->arrForm, false ,true); 
     65        } 
     66        break; 
     67 
     68    case 'delete': 
     69        $this->doDelete($this->arrForm); 
     70        break; 
     71 
     72    case 'disp': 
     73        /** NG ビジネスロジックの記述をしてはならない */ 
     74        foreach ($this->arrForm as $key => $val) { 
     75            $foo[$key] = $val; 
     76        } 
     77 
     78        /** NG SQLを実行する処理を記述してはならない */ 
     79        $objQuery =& SC_Query::getSingletonInstance(); 
     80        $arrResults = $objQuery->select("*", "table_name"); 
     81 
     82        $this->doDisp($this->arrForm); 
     83        break; 
     84 
     85    case 'complete': 
     86        $this->registerProductClass($this->arrForm, $this->arrForm['product_id']); 
     87        SC_Response_Ex::sendRedirect("complete.php"); 
     88        break; 
     89 
     90    default: 
     91    } 
    8992} 
    9093?> 
    … …  
    98101#!php 
    99102<?php 
    100 // OK 
     103/** OK switch 文で mode の分岐を行う */ 
    101104switch ($this->getMode()) { 
    102      case 'disp': 
    103          $this->doDisp($this->arrForm); 
    104          break; 
    105  
    106      case 'complete': 
    107          SC_Response_Ex::sendRedirect("complete.php"); 
    108          break; 
    109  
    110      default: 
    111      } 
    112 } 
    113  
    114 // NG 
     105case 'disp': 
     106    $this->doDisp($this->arrForm); 
     107    break; 
     108 
     109case 'complete': 
     110    SC_Response_Ex::sendRedirect("complete.php"); 
     111    break; 
     112default: 
     113} 
     114 
     115/** NG mode の条件分岐に if 文を使用してはならない */ 
    115116if ($_POST['mode'] == 'disp') { 
    116      $this->doDisp($this->arrForm); 
     117    $this->doDisp($this->arrForm); 
    117118} elseif ($_POST['mode'] == 'complete') { 
    118      SC_Response_Ex::sendRedirect("complete.php"); 
    119 } 
    120 ?> 
    121 }}} 
     119    SC_Response_Ex::sendRedirect("complete.php"); 
     120} 
     121?> 
     122}}} 
     123 
    122124== ビジネスロジック == 
    123125 
    124   * ページ間で重複する処理が存在する場合は, Helper などの共通クラスへ 記述する 
     126  * ページ間で重複する処理が存在する場合は, Helper などの共通クラスへ記述する 
    125127  * ページ固有の処理は, ローカル関数で記述する 
     128  * 可能であれば, PHPUnit を使用してテストケースを残すこと 
    126129  * 宣言を除き, 引数や返り値が無く, すべて内部のメンバ変数で処理するような関数は極力作成しない 
    127130    * ステートレスな処理を心掛けること 
    … …  
    130133#!php 
    131134<?php 
    132 // OK 
     135 
     136/** 
     137 * NG 引数ではなく, クラスのメンバ変数を使用して振舞いを変更する 
     138 */ 
     139function lfSendMail() { 
     140     $objPurchase = new SC_Purchase_Ex(); 
     141     $objPurchase->sendOrderMail($this->order_id) 
     142} 
     143 
     144/** 
     145 * OK 引数で振舞いを変更できる 
     146 */ 
    133147function lfSendMail($order_id) { 
    134148     $objPurchase = new SC_Purchase_Ex(); 
    … …  
    136150} 
    137151 
    138 // NG 
    139 function lfSendMail() { 
    140      $objPurchase = new SC_Purchase_Ex(); 
    141      $objPurchase->sendOrderMail($this->order_id) 
    142 } 
    143 ?> 
    144 }}} 
    145  
    146   * 可能であれば, PHPUnit を使用してテストケースを残すこと 
    147   * ループ処理などで, メンバ変数や, スーパーグローバル変数は直接使わない 
    148  
    149 {{{ 
    150 #!php 
    151 <?php 
    152 // OK 
     152/** 
     153 * NG 引数や返り値が無く, メンバ変数の値を直接変更している 
     154 */ 
     155function lfConvertLoginPass(){ 
     156    if(strlen($this->arrForm["login_pass"]) < 1 ) { 
     157        return; 
     158    } 
     159    $this->arrForm["login_pass"] = trim($this->arrForm["login_pass"]); 
     160    $this->arrForm["login_pass1"] = $this->arrForm["login_pass"]; 
     161    $this->arrForm["login_pass2"] = $this->arrForm["login_pass"]; 
     162} 
     163 
     164/** 
     165 * OK ローカル関数内では, ローカル変数を使用することで, 拡張性, 保守性が向上し, ユニットテストも書きやすい 
     166 */ 
     167 
     168$this->arrForm = $this->lfConvertLoginPass($this->arrForm); 
     169 
     170function lfConvertLoginPass(&$arrForm){ 
     171    if(strlen($arrForm["login_pass"]) < 1 ) { 
     172        return; 
     173    } 
     174    $arrForm["login_pass"] = trim($arrForm["login_pass"]); 
     175    $arrForm["login_pass1"] = $arrForm["login_pass"]; 
     176    $arrForm["login_pass2"] = $arrForm["login_pass"]; 
     177    return $arrForm; 
     178} 
     179 
     180/** 
     181 * ユニットテストの例. 
     182 */ 
     183function testLfConvertLoginPass() { 
     184    $login_pass = "login_pass_value"; 
     185    $arrForm = array("login_pass" => $login_pass); 
     186 
     187    $expected = array("login_pass" => $login_pass, 
     188                      "login_pass1" => $login_pass, 
     189                      "login_pass2" => $login_pass); 
     190 
     191    $actual = lfConvertLoginPass($arrForm); 
     192 
     193    $this->assertEquals($expected, $actual); 
     194} 
     195?> 
     196}}} 
     197 
     198 * ループ処理などで, メンバ変数や, スーパーグローバル変数は直接使わない 
     199 
     200{{{ 
     201#!php 
     202<?php 
     203 
     204/** 
     205 * NG ループ処理で, メンバ変数やスーパーグローバル変数を直接扱っている 
     206 */ 
     207foreach ($_POST['params'] as $key => $val) { 
     208     $this->arrForm[$key] = $val; 
     209} 
     210 
     211/** 
     212 * OK メンバ変数や, スーパーグローバル変数に作用するループ処理は, 別途関数を作成する 
     213 */ 
    153214$this->arrParams = $this->getParams($_POST['params']); 
    154215 
    … …  
    161222} 
    162223 
    163 // NG 
    164 foreach ($_POST['params'] as $key => $val) { 
    165      $this->arrForm[$key] = $val; 
    166 } 
    167224?> 
    168225}}} 
    … …  
    170227== データベースアクセス == 
    171228 
    172   * SC_Query のインスタンスを毎回必ず生成する必要の無い場合は SC_Query::getSingletonInstance() を使用してインスタンスを取得すること 
    173   * SQL 文を散乱させない 
    174     * SELECT id, name, foo, bar FROM table_name と SELECT id, name, foo, bar, too FROM table_name が存在する場合は, 後者を共通関数にリファクタリングすること 
    175   * INSERT/UPDATE/DELETE は, SC_Query::insert(), SC_Query::update(),  SC_Query::delete() を使用し, SC_Query::query() は使用しないこと 
    176  
     229特に理由の無い場合は `SC_Query::getSingletonInstance()` を使用してインスタンスを取得すること 
     230 
     231{{{ 
     232#!php 
     233<?php 
     234 
     235/** 
     236 * NG 
     237 * PHP4 環境では, 新たなインスタンスが生成されてしまう 
     238 */ 
     239$objQuery = new SC_Query(); 
     240 
     241/** 
     242 * OK シングルトンが保証され, トランザクションの扱いが容易になる 
     243 */ 
     244$objQuery =& SC_Query::getSingletonInstance(); 
     245 
     246?> 
     247}}} 
     248 
     249SQL 文を散乱させない. 
     250SELECT id, name, foo, bar FROM table_name と SELECT id, name, foo, bar, too FROM table_name が存在する場合は, 後者を共通関数にリファクタリングすること 
     251 
     252{{{ 
     253#!php 
     254<?php 
     255 
     256/** 
     257 * NG 類似した SQL を散乱させてはいけない 
     258 */ 
     259$arrResults = $objQuery->select("id, name, foo, bar", "table_name"); 
     260$arrResult2 = $objQuery->select("id, name, foo, bar, too", "table_name", "too = ?", $arrParams['too']); 
     261 
     262/** 
     263 * OK カラム名や, WHERE の違いなどは共通関数で吸収する 
     264 */ 
     265function getResults($arrParams = array()) { 
     266    $where = ""; 
     267    $arrValues = array(); 
     268    if (isset($arrParams['too'])) { 
     269        $where .= "too = ?"; 
     270        $arrValues[] = $arrParams['too']; 
     271 
     272    } 
     273    $objQuery =& SC_Query::getSingletonInstance(); 
     274    $arrResults = $objQuery->select("*", "table_name", $where, $arrValues); 
     275} 
     276$arrResults = getResults(); 
     277$arrResults2 = getResults($arrParams); 
     278 
     279?> 
     280}}} 
     281 
     282INSERT/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}}} 
    177299== 入力チェック == 
    178300 
    … …  
    181303== スーパーグローバル変数 == 
    182304 
    183 $_POST や $_GET の値は, 必ず入力チェック後に使用する. SC_FormParam を使用していれば, SC_FormParam::getHashArray() で取得できる 
    184  
    185 {{{ 
    186 #!php 
    187 <?php 
     305`$_POST` や `$_GET` の値は, 必ず入力チェック後に使用する. `SC_FormParam` を使用していれば, `SC_FormParam::getHashArray()` で取得できる 
     306 
     307{{{ 
     308#!php 
     309<?php 
     310 
     311/** NG $_POST の値を入力チェックせずに使用する */ 
     312$this->register($_POST); 
     313 
     314/** OK 必ず入力チェックを行う */ 
    188315$objFormParam = new SC_FormParam(); 
    189316$objFormParam->addParam("商品規格ID", "product_class_id", INT_LEN, "n  
    … …  
    196323     $arrForm = $objFormParam->getHashArray(); 
    197324} 
    198 ?> 
    199 }}} 
    200  
    201 `$_SESSION` は, SC_CartSession クラス等, セッションを扱うビジネスロジックを通じて使用すること. 
     325$this->register($arrForm); 
     326?> 
     327}}} 
     328 
     329`$_SESSION` は, `SC_CartSession クラス`等, セッションを扱うビジネスロジックを通じて使用するのが望ましい. 
    202330 
    203331どうしても, グローバル変数を使用したい場合は, global キーワードは使用せず, `$GLOBALS` を使用すること. 
    204332 
     333{{{ 
     334#!php 
     335<?php 
     336/** 
     337 * NG global キーワードの使用すると, 変数名衝突の原因となる 
     338 */ 
     339function globalSample() { 
     340    global $conn; 
     341 
     342    // 外部からは $conn という変数を使用しているのがわからない 
     343    if (is_null($conn)) { 
     344        $conn =& new ClassName(); 
     345    } 
     346} 
     347 
     348/** 
     349 * OK $GLOBALS を使用すると, ローカル変数との衝突の心配は無い 
     350 */ 
     351function globalSample() { 
     352    if (is_null($GLOBALS['_conn'])) { 
     353        $GLOBALS['_conn'] = new ClassName(); 
     354    } 
     355} 
     356 
     357?> 
     358}}} 
     359 
    205360== 変数名 == 
    206361 
    … …  
    211366<?php 
    212367 
    213 /** 違反サンプル */ 
     368/** 
     369 * NG 一見して, どんなデータが格納されているかわからない 
     370 */ 
    214371$arrRet = getProducts(); 
    215372 
    216 /** 修正サンプル */ 
     373/** 
     374 * OK 商品データの配列だと, 変数名で判別可能 
     375 */ 
    217376$arrProducts = getProducts(); 
    218377}}} 
    … …  
    221380 
    222381{{{ 
    223 誤) 
    224 $productId 
    225  
    226 正) 
    227 $product_id 
    228 }}} 
    229  
     382#!php 
     383<?php 
     384 
     385/** 
     386 * テーブルカラム名と共通の変数名は, 連想配列の添字で使用されるため, 
     387 * アンダーバー区切りの方が扱いやすい 
     388 */ 
     389 
     390/** 
     391 * NG 
     392 */ 
     393$productId = getProductId(); 
     394 
     395/** 
     396 * OK 
     397 */ 
     398$product_id = getProductId(); 
     399}}} 
     400 
     401== 1行の文字数/行数 == 
     402 * 1行の行数は, 80文字を目安に折り返すのが望ましい 
     403 * 1関数の行数は多くても200行程度に留めるのが望ましい. これ以上行数が増えてしまう場合は, 関数を分割する等, コーディングの見直しを行ってください 
    230404== PHPDoc コメント == 
     405 
    231406関数の PHPDoc コメントは必ず記述する. 
    232407  * 関数の説明 
    233408  * 引数 
     409    * 引数の無い関数は省略可能 
    234410  * 返り値 
     411    * 返り値が無くても void を明記すること 
    235412 
    236413== 非推奨機能 == 
    237414 
    238 error_reporting(E_ALL) で, エラー及び警告が出力されないようコーディン  
    239 グすること. 
     415error_reporting(E_ALL) で, エラー及び警告が出力されないようコーディングすること. 
     416 
    240417PHP5.3 で非推奨となっている関数は使用しないこと. 
    241418 
    … …  
    276453== 不要な機能 == 
    277454 
    278 TODO 
    279  
     455#793 に該当する機能は削除すること 
     456http://svn.ec-cube.net/open_trac/ticket/793