devrait cette méthode être décomposée en méthodes séparées?

Question

Cette méthode recherche un mot-clé de recherche et une requête mysql analysée, et réécrit l'expression where pour inclure LIKE% keyword%.

Il fonctionne bien, mais je ne sais pas si sa pratique bonne ou mauvaise d'avoir une méthode avec ce nombre de boucles ...

private function build_where($query_array, $options) 
{ 
    //add WHERE starting point 
    $where = '';   

    if(!empty($query_array['WHERE'])) 
    { 
     //build where array 
     $where_array = $query_array['WHERE']; 

     //start the where 
     $where .= 'WHERE '; 

     //get columns array 
     $columns_array = $this->build_columns_array($query_array); 

     //if there is a search string   
     if(!empty($options['sSearch'])) 
     { 
      //check for enabled columns 
      $i = 0; 
      $columns_length = count($columns_array); 
      for($i; $i < intval($columns_length); $i++) 
      { 
       //create the options boolean array 
       $searchable_columns['bSearchable_'.$i] = $options['bSearchable_'.$i]; 
      } 

      //loop through searchable_columns for true values 
      foreach($searchable_columns as $searchable_column_key => $searchable_column_val) 
      { 
       if($searchable_column_val == true) 
       { 
        //get an integer from the searchable_column key 
        $column_id = preg_replace("/[^0-9]/", '', $searchable_column_key); 

        //lookup column name by index 
        foreach($columns_array as $columns_array_key => $columns_array_val) 
        { 
         //if the $columns_array_key matches the $column_id 
         if($columns_array_key == $column_id) 
         { 
          //loop to build where foreach base expression 
          $i = 0; 
          $where_length = count($where_array); 
          for($i; $i < intval($where_length); $i++) 
          { 
           //append the existing WHERE Expressions 
           $where .= $where_array[$i]['base_expr']; 
          }        

          //append the LIKE '%$options['sSearch'])%' 
          $where .= ' AND '.$columns_array_val." LIKE '%".$options['sSearch']."%' OR "; 
         } 
        } 
       } 
      } 
      //remove the last OR 
      $where = substr_replace($where, "", -3);          
     } 
     else 
     { 
      //loop to build where 
      $i = 0; 
      $where_length = count($where_array); 
      for($i; $i < intval($where_length); $i++) 
      { 
       $where .= $where_array[$i]['base_expr']; 
      } 
     }    
    } 

    //print_r($where_length); 
    return $where; 
} 

Answer 1

Les méthodes de décomposition ne concernent pas principalement la réutilisation. Cela peut faciliter la lecture, le test et la maintenance du code. Les noms de méthode Clear peuvent également remplacer les commentaires en ligne. Cette méthode fait deux choses de haut niveau qui pourraient être séparées: construire une clause where avec des options et sans options. Un autre indice pour moi est que la logique qui construit la clause where avec les options semble assez riche pour justifier sa propre méthode.

private function build_where($query_array, $options) { 
    if(!empty($query_array['WHERE'])) { 
     $where_array = $query_array['WHERE']; 
     $columns_array = $this->build_columns_array($query_array); 
     if (empty($options['sSearch'])) { 
      return $this->build_where_with_options($where_array, $columns_array, $options); 
     } 
     else { 
      return $this->build_where_without_options($where_array, $columns_array); 
     } 
    } 
    else { 
     return ''; 
    } 
} 

Maintenant, vous pouvez parcourir rapidement build_where() pour voir qu'il ya trois formes possibles de la clause where peut prendre et quand ainsi que l'entrée de chaque formulaire doit produire son résultat.

Voici quelques améliorations mineures, vous pouvez faire tout au long de votre code:

• count() retourne un entier et n'a pas besoin des intval() appels dans vos for boucles. Même si vous les avez laissés, il serait préférable d'appliquer l'appel en dehors de la boucle, donc c'est fait une seule fois car il donne la même valeur à chaque fois.
• if($searchable_column_val == true) est équivalent à if($searchable_column_val) puisque les deux cast $searchable_column_val à une valeur booléenne et le dernier passe lorsque cette valeur booléenne casted est égale à true.
• $where = substr_replace($where, "", -3) peut être remplacer par $where = substr($where, 0, -3) et est un peu plus clair. Au lieu de faire défiler un tableau à la recherche d'une clé spécifique, vous pouvez profiter des tableaux de PHP en saisissant simplement la valeur avec cette clé.

Pour le dernier, ce code

foreach($columns_array as $columns_array_key => $columns_array_val) 
{ 
    //if the $columns_array_key matches the $column_id 
    if($columns_array_key == $column_id) 
    { ... } 
} 

peut être remplacé par ce

$columns_array_val = $columns_array[$column_id]; 
... 

Answer 2

préférence personnelle vraiment. Certains programmeurs couperaient ceci en plusieurs fonctions. Personnellement, je pense que c'est comme ça que vous l'avez. Si je voyais quelque chose que je pensais être réutilisable, je le refactoriserais dans un fichier séparé qui pourrait être inclus. À mon avis, certains programmeurs sont trop rapides pour rendre les choses «réutilisables» avant même d'avoir quelque chose à réutiliser.

Answer 3

L'école de pensée de Kent Beck ou de Martin Fowler vous conseillerait de refactoriser ces grandes méthodes à de nombreuses petites méthodes. Ce n'est pas facile à lire selon moi, ce qui serait la principale raison pour refactoriser.