2011-09-07 2 views
5

J'ai rencontré ce code dans lequel un appel de méthode, par exemple ClassA.search (a, b, flag) est utilisé par 3 contrôleurs. Ceci est une version simplifiée de la méthode:Est-ce un bon moyen de réutiliser/partager une méthode?

public List<Result> search(Object a, Object b, boolean flag) { 
    //do some code logic here, common to the 3 controllers 
    //at the middle there is: 
    if (flag) { 
     //code that affects 2 Controllers 
    } else { 
     //code affects only 1 
    } 
    //some more common code 
    //some more code with the flag if else 
} 

Est-ce une bonne idée parce que le code est réutilisé? Ou y a-t-il un meilleur moyen de réutiliser le code mais de ne pas introduire ce drapeau pour la personnalisation du code de l'appelant de méthode (comme peut-être le diviser en 3 méthodes différentes mais être capable de déclarer une méthode refactorisée)?

Répondre

6

D'abord, extrait les lignes de commentaires avec des fonctions:

public void search(Object a, Object b, boolean flag) 
{ 
    commonToThree(); 
    if (flag) 
    { 
     affectTwoControllers(); 
    } 
    else 
    { 
     affectsOnlyOne(); 
    } 
    alsoCommon(); 
} 

maintenant se débarrasser de flag argument booléen, qui est une odeur de code:

public void searchWithTrueFlag(Object a, Object b) { 
    commonToThree(); 
    affectTwoControllers(); 
    alsoCommon(); 
} 

public void searchWithFalseFlag(Object a, Object b) { 
    commonToThree(); 
    affectsOnlyOne(); 
    alsoCommon(); 
} 
+0

Je suis d'accord avec cela à condition de ne pas finir par transformer des variables locales en champs pour le faire fonctionner. –

+0

Pourquoi trouvez-vous les variables locales mauvaises? Si l'état que vous devez passer à travers les paramètres est si important, créez un objet unique avec l'état initialisé aux champs finaux dans le constructeur et ne l'utilisez qu'une seule fois. Fonction sur les stéroïdes ;-). –

+0

C'est vrai mais ajouter une classe avec des champs (et un constructeur?) Demande beaucoup de travail pour éviter un drapeau. ;) –

3

C'est bon mais pas génial. Un booléen a du sens, mais si vous commencez à en ajouter d'autres, vous n'irez pas dans la bonne direction.

Il est pas toujours possible, mais donne généralement un meilleur code à faire:

functionOne: 
    sharedCodeOne() 
    specificCode 
    sharedCodeTwo() 

functionTwo: 
    sharedCodeOne() 
    specificCode 
    sharedCodeTwo() 

Comme toujours, il est difficile de faire des réclamations généralisées: ce n'est évidemment pas toujours possible/pratique.

+1

+1: C'est bien si les trois sections du code ne partagent aucune variable locale.(ou seulement partager un petit nombre) –

+1

@Peter: Je suis tout à fait d'accord - Je pense que c'est réellement et avantage de cette approche qui vous conduit à réduire le nombre et la portée des variables locales. (Bien que certaines personnes pourraient avoir tendance à les transformer en champs de la classe englobante, en faisant un tas de choses) –

+1

@Arnout Engelen, je suis d'accord que l'ajout de champs pour faire ce travail n'est pas une bonne idée. –

0

C'est une manière relativement simple de le faire. Il existe des alternatives mais elles sont susceptibles d'être plus complexes. (Comme passer un visiteur ou appeler une méthode du contrôleur pour dire quoi faire pour ce contrôleur)

Cette approche est préférable si vous partagez des variables locales entre les trois sections de code et vous devrez utiliser des champs à la place.

0

je prendrais une approche différente en essayant de répondre cette question d'une manière générale:

L'objectif principal devrait être propre code. Qu'est-ce que c'est exactement, bien sûr, dépend du cas spécifique. Mais à coup sûr, il est mauvais d'avoir du code copié et collé à plusieurs endroits, car cela nécessite de changer plusieurs endroits si cela doit être des changements - aussi, il est mauvais d'essayer d'extraire des parties communes là où plus d'une partie utilise eux en toutes circonstances.

Imaginez toujours que quelqu'un doive lire votre code (ou que vous ayez à le faire dans quelques semaines) et comprendre le plus rapidement possible ce qui s'y passe. Donc, cela dépend du cas particulier, s'il vaut mieux copier certaines lignes ou les extraire dans une méthode commune.

Un indicateur n'est pas mauvais en soi, mais ce n'est pas vraiment quelque chose qui devrait être trop utilisé dans les méthodes java. Souvent, une telle situation peut être résolue facilement en surchargeant une méthode et en utilisant l'une dans l'autre, de sorte que le cas commun soit fait dans l'une et l'addition spéciale dans l'autre. Alternativement, vous pouvez avoir plusieurs sous-méthodes et composer votre méthode avec plusieurs appels, mais cela n'a de sens que si vous n'avez pas besoin de passer trop de paramètres. Une règle (complètement subjective, mais basée sur les leçons tirées de nombreux projets) Je peux vous donner: Favoriser les implémentations concrètes par des approches génériques. Ils peuvent générer plus de code et peuvent sembler moins intelligents, mais ils sont beaucoup plus faciles à comprendre, à étendre et à déboguer. +1:

Questions connexes