2009-12-16 4 views
0

J'ai une question rapide sur refactoring php code. Voici trois fonctions. Les deux premiers semblent assez similaires et ne diffèrent que par une déclaration if. Le troisième combine les deux premiers grâce à l'utilisation d'un drapeau. Est-ce la meilleure pratique? Ici, il semble correct d'utiliser un drapeau, mais que se passe-t-il si nous devons ajouter plus de drapeaux dans le futur? Quelle est la meilleure pratique?Question de refactorisation codeigniter simple - meilleure pratique

Merci.

function check_contact_email($email) 
{ 
    $this->db->select('COUNT(login) AS count'); 
    $this->db->from('users'); 
    $this->db->where('email', $email); 
    $query = $this->db->get(); 
    $row = $query->row(); 
    return ($row->count > 0); 
} 

function check_contact_email_id($email) 
{ 
    $this->db->select('COUNT(login) AS count'); 
    $this->db->from('users'); 
    $this->db->where('email', $email); 
    $this->db->where('user_id !=', $_POST['user_id']); 
    $query = $this->db->get(); 
    $row = $query->row(); 
    return ($row->count > 0); 
} 

function check_contact_email($email, $id = FALSE) 
{ 
    $this->db->select('COUNT(login) AS count'); 
    $this->db->from('users'); 
    $this->db->where('email', $email); 
    if ($id) $this->db->where('user_id !=', $_POST['user_id']); 
    $query = $this->db->get(); 
    $row = $query->row(); 
    return ($row->count > 0); 
} 
+0

Quelle version de gâteau utilisez-vous? ce n'est pas 'n'importe où même proche de la bonne façon de le faire ... – user103219

+0

désolé * c'est codeigniter (j'ai eu mes projets mélangés). Que suggérez-vous? – Dirk

+0

ahh ok, eh bien dans ce cas, je ne suis pas familier avec le codeigniter ... dans cakephp toutes ces fonctions peuvent être combinées en une seule fonction $ this-> Model-> Find() – user103219

Répondre

5

Tout d'abord, vous pouvez réduire tout cela à l'aide des moins connus (mais documentés) méthodes ActiveRecord comme ceci:

function check_contact_email($email) 
{ 
    $this->db->where('email', $email); 
    return $this->db->count_all_results('users') > 0; 
} 

function check_contact_email_id($email) 
{ 
    $this->db->where('user_id !=', $_POST['user_id']); 
    return $this->check_content_email($email); 
} 

function check_contact_email($email, $id = FALSE) 
{ 
    if ($id) $this->db->where('user_id !=', $_POST['user_id']); 
    return $this->check_content_email($email); 
} 

Vous pouvez réduire ce plus en passant un tableau pour les drapeaux:

function check_contact_email($params) 
{ 
    if(is_array($params)) 
    { 
     $this->db->where($params); 
    } 

    else 
    { 
     $this->db->where('email', $params); 
    } 

    return $this->db->count_all_results('users') > 0; 
} 

Avec que vous avez une fonction qui peut agir de différentes façons:

$this->your_model->check_contact_email($email); 

$this->your_model->check_contact_email(array(
    'email' => $email, 
    'id !=' => $this->input->post('user_id') 
)); 

$this->your_model->check_contact_email(array(
    'email' => $email, 
    'id !=' => $this->input->post('user_id'), 
    'otherfield' => $whatever 
)); 

Ce n'est pas parfait MVC de mettre cette logique de base de données TINY (le! =) Dans votre contrôleur, mais c'est tout aussi mauvais de mettre les données de formulaire directement dans les fonctions de votre modèle, alors choisissez celui qui vous semble le plus flexible.

0

La troisième fonction prend la place des deux premiers. Mais si vous prévoyez d'avoir plus de drapeaux, je pense que vous voudriez faire une méthode plus générique. Ce qui semble changer entre les méthodes est la clause where, donc si vous faites une méthode avec la clause where paramétrée, cela gérera cette situation. Note: Je ne connais pas du tout le php, je me base simplement sur l'aspect du code.

Questions connexes