2010-08-03 6 views
1

Je suis en train de refactoriser un ancien code. Je cherche des directions sur le meilleur modèle de conception à utiliser ici. Je pense au modèle d'usine mais je ne suis pas sûr que ce soit la meilleure façon d'y aller ou pas.Comment refactoriser ce code

Voici donc un bref aperçu du pseudocode. La classe Foo a la logique métier de base.

Class Foo{ 
    private List<Things> stuff; 
    private static Integer count; 
    //getter setter for stuff 

    public Foo(List<Things> stuff){ 
     this.stuff = stuff; 
     this.count=1; 
    } 

    //the following 3 methods are 90% similar 

    public List<Newthings> doSomeThingFirst(){ 
     //uses the list stuff and also increments the static member count for each entry in List<NewThings> based on certain conditions 
    } 

    public List<Newthings> doSomethingSecond(){ 
     //uses the list stuff and also increments the static member count for each entry in List<NewThings> based on certain conditions 
    } 

    public List<Newthings> doSomethingThird(){ 
     //uses the list stuff and also increments the static member count for each entry in List<NewThings> based on certain conditions 
    } 

    //in the future there may be doSomethingFourth(), doSomethingFifth() ... etc. 

} 


The caller of class Foo looks something like below. 

Class SomeServiceImpl{ 
    public List<Things> getAllFoo(List<Things> stuff){ 
     Map<Integer,NewThings> fooList = new HashMap<Integer,NewThings>(); 
     Foo foo = new Foo(stuff); 
     fooList.put(1,foo.doSomeThingFirst()); 
     fooList.put(2,foo.doSomeThingSecond()); 
     fooList.put(3,foo.doSomeThingThird()); 
      return new ArrayList<Things>(fooList.values()); 

    } 
} 

Permettez-moi de savoir comment pensez-vous que ce code devrait être refactorisé pour maintenabilité et de réutiliser ou est-ce bien comme il est?

Merci pour vos contributions.

+0

Mon Java est rouillé, mais ArrayList prend deux arguments de type? Que diable pour? –

+0

Mon mauvais. J'essayais de faire un code pseudo sur le code actuel et j'ai tapé une liste au lieu d'une carte. – CoolBeans

+0

Impossible de comprendre votre code. Aussi, il semble y avoir beaucoup d'erreurs de codage – YoK

Répondre

5
// the following 3 methods are 90% similar 

Puisque vous ne l'avez pas le code d'offre réelle, qui est la partie que vous factoriser.

+1

DRY - Ne vous répétez pas - Faites les choses une fois et une fois seulement, ne possédez pas de code similaire à 98%. +1 –

1

tout d'abord vous devez remplacer la liste avec la carte

Class SomeServiceImpl{ 
    public getAllFoo(List<Things> stuff){ 
     Map<Integer,NewThings> fooMap = new HashMap<Integer,NewThings>(); 
     Foo foo = new Foo(stuff); 
     fooMap.add(1,foo.doSomeThingFirst()); 
     fooMap.add(2,foo.doSomeThingSecond()); 
     fooMap.add(3,foo.doSomeThingThird()); 

    } 
} 

La prochaine chose est que l'exemple est vraiment pores avec la logique dehors et il est difficile de comprendre ce que vous voulez vraiment faire.

La classe SomeServiceImpl effectue certaines opérations sur la liste mais sous la valeur 1,2,3 vous avez toujours le même résultat. Vous effectuez l'opération tout en ajoutant à la liste pas lorsque vous récupérez la liste de cette carte.

Je pense que vous avez choisi le motif de conception incorrect, à la place fatory Vous devez utiliser le modèle Builder.

http://en.wikipedia.org/wiki/Builder_pattern#Java

1

Je dirais que le facteur de 90% dans une méthode publique qui fournit les services à l'appelant, et ensuite des méthodes d'aide privées pour prendre soin de doFirst(), doSecond(), etc. De cette façon, si vous en ajoutez d'autres, votre API publique ne changera pas et ne cassera pas les clients.

2

Le modèle de commande pourrait aider ici. C'est un cas typique d'application du principe "Encapsulate that varie". Le code serait:

public interface Command { 
    public List<NewThings> doSomething(); 
} 

class DoSomethingFirst implements Command { 
    public DoSomethingFirst(Foo foo) { 
    ... 
    } 
    public List<NewThings> doSomething() { 
    ... 
    } 
} 

alors votre code de service sera

fooList.put(1,(new DoSomeThingFirst(foo)).doSomething()); 
    fooList.put(2,(new DoSomeThingSecond(foo)).doSomething()); 
    fooList.put(3,(new DoSomeThingThird(foo)).doSomething()); 

ainsi les méthodes Foo peuvent être enlevés et déléguer vos actions à des cours. Ceci est mon point de vue plus flexible que l'approche précédente, car vous pouvez ajouter autant d'actions - même au moment de l'exécution.

Si les classes dérivées de commande ont une grande partie du code similaire faire quelque chose comme:

public abstract class DoSomethingBasic implements Command { 
... 
// common code here. 
... 
} 

public class DoSomethingFirst extends DoSomething { 
    public list<NewThings> doSomething() { 
    super.doSomething(); //if possible 
    ... 
    } 
} 

mes 2 cents.

+0

Intéressant. Je n'ai pas été exposé à ce modèle dans le passé. Bon à savoir. Merci pour votre aide mate. – CoolBeans

Questions connexes