Comment rendre ce code C# petit ou refactoriser ce code

Question

Je suis nouveau avec le codage et j'ai récemment lu un article sur le refactoring de code. J'ai donc fait une application console pour réserver des chambres dans un bateau. Je pense que dans mon projet il y a seulement 2 parties où j'ai besoin de refactoring, qui sont les suivantes. L'une est l'instruction else else.

 ship1 = new Ship("Olympic Countess"); 

     ArrayList groupA = new ArrayList(); 
     for (int i = 0; i < 10; i++) 
     { 
      groupA.Add(new room(5000, "A" + (i + 1))); 
     } 

     ArrayList groupB = new ArrayList(); 
     for (int i = 0; i < 10; i++) 
     { 
      groupB.Add(new room(4000, "B" + (i + 1))); 
     } 

     ArrayList groupC = new ArrayList(); 
     for (int i = 0; i < 30; i++) 
     { 
      groupC.Add(new room(3500, "C" + (i + 1))); 
     } 

     ArrayList groupD = new ArrayList(); 
     for (int i = 0; i < 36; i++) 
     { 
      groupD.Add(new room(3400, "D" + (i + 1))); 
     } 

     ArrayList groupE = new ArrayList(); 
     for (int i = 0; i < 40; i++) 
     { 
      groupE.Add(new room(3300, "E" + (i + 1))); 
     } 

     ArrayList groupF = new ArrayList(); 
     for (int i = 0; i < 30; i++) 
     { 
      groupF.Add(new room(3400, "F" + (i + 1))); 
     } 

     ArrayList groupG = new ArrayList(); 
     for (int i = 0; i < 36; i++) 
     { 
      groupG.Add(new room(3300, "G" + (i + 1))); 
     } 

     ArrayList groupH = new ArrayList(); 
     for (int i = 0; i < 40; i++) 
     { 
      groupH.Add(new room(3200, "H" + (i + 1))); 
     } 

     ship1.addDeck("Balcony Suite", groupA); 
     ship1.addDeck("Suite", groupB); 
     ship1.addDeck("Deck 3 - Outside Twin", groupC); 
     ship1.addDeck("Deck 2 - Outside Twin", groupD); 
     ship1.addDeck("Deck 1 - Outside Twin", groupE); 
     ship1.addDeck("Deck 3 - Inside Twin", groupF); 
     ship1.addDeck("Deck 2 - Inside Twin", groupG); 
     ship1.addDeck("Deck 1 - Inside Twin", groupH);  
    } 

et l'autre est if else comme suit

public Reservation bookPassage(String cabinclass, Customer booker, int number) 
     { 
     ArrayList cabins; 
     if (cabinclass.Equals("a", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Balcony Suite"); 
     else if (cabinclass.Equals("b", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Suite"); 
     else if (cabinclass.Equals("c", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Deck 3 - Outside Twin"); 
     else if (cabinclass.Equals("d", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Deck 2 - Outside Twin"); 
     else if (cabinclass.Equals("e", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Deck 1 - Outside Twin"); 
     else if (cabinclass.Equals("f", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Deck 3 - Inside Twin"); 
     else if (cabinclass.Equals("g", StringComparison.OrdinalIgnoreCase)) 
      cabins = ship1.getDeck("Deck 2 - Inside Twin"); 
     else 
      cabins = ship1.getDeck("Deck 1 - Inside Twin"); 

Ce que je ne comprends pas que mes paramètres changent dans les deux logiques. Alors, comment puis-je faire une méthode distincte pour cette logique lorsque mon cours de cabine change à chaque fois?

Answer 1

Toutes vos itérations peuvent être reliés entre eux, donc exécuté en une seule fois comme:

ArrayList groupA = new ArrayList(); 
    ArrayList groupB = new ArrayList(); 
    ArrayList groupC = new ArrayList(); 
    ArrayList groupD = new ArrayList(); 
    ArrayList groupE = new ArrayList(); 
    ArrayList groupF = new ArrayList(); 
    ArrayList groupG = new ArrayList(); 
    ArrayList groupH = new ArrayList(); 

    for (int i = 0; i < 40; i++) 
    {   
     if (i < 10) 
     { 
      groupA.Add(new room(5000, "A" + (i + 1))); 
      groupB.Add(new room(4000, "B" + (i + 1))); 
     } 
     if (i < 30) 
     { 
      groupF.Add(new room(3400, "F" + (i + 1))); 
      groupC.Add(new room(3500, "C" + (i + 1))); 
     } 
     if (i < 40) 
     { 
      groupE.Add(new room(3300, "E" + (i + 1))); 
      groupH.Add(new room(3200, "H" + (i + 1)));   
     } 
     if (i < 36) 
     { 
      groupD.Add(new room(3400, "D" + (i + 1))); 
      groupG.Add(new room(3300, "G" + (i + 1))); 
     }    
    } 

    ship1.addDeck("Balcony Suite", groupA); 
    ship1.addDeck("Suite", groupB); 
    ship1.addDeck("Deck 3 - Outside Twin", groupC); 
    ship1.addDeck("Deck 2 - Outside Twin", groupD); 
    ship1.addDeck("Deck 1 - Outside Twin", groupE); 
    ship1.addDeck("Deck 3 - Inside Twin", groupF); 
    ship1.addDeck("Deck 2 - Inside Twin", groupG); 
    ship1.addDeck("Deck 1 - Inside Twin", groupH); 

et le bouquet de mutuellement exclusifs ifs peuvent être joints dans un lik cse:

switch (cabinclass.ToLower()) 
{ 
    case "a": 
     cabins = ship1.getDeck("Balcony Suite"); 
     break; 
    case "b": 
     cabins = ship1.getDeck("Suite"); 
     break; 
    case "c": 
     cabins = ship1.getDeck("Deck 3 - Outside Twin"); 
     break; 
    case "d": 
     cabins = ship1.getDeck("Deck 2 - Outside Twin"); 
     break; 
    case "e": 
     cabins = ship1.getDeck("Deck 1 - Outside Twin"); 
     break; 
    case "f": 
     cabins = ship1.getDeck("Deck 3 - Inside Twin"); 
     break; 
    case "g": 
     cabins = ship1.getDeck("Deck 2 - Inside Twin"); 
     break; 
    default: 
     cabins = ship1.getDeck("Deck 1 - Inside Twin"); 
     break;    
} 

Answer 2

Utilisez une fonction paramétrée.

E.g. dans

ArrayList groupC = new ArrayList(); 
    for (int i = 0; i < 30; i++) 
    { 
     groupC.Add(new room(3500, "C" + (i + 1))); 
    } 

les éléments diffèrent sont: groupC, 30, 3500 et "C".

Effectuez une méthode avec les éléments ci-dessus comme argument et appelez cette méthode avec les paramètres corrects pour chaque fragment.

Answer 3

En regardant ce morceau de code, il y a probablement beaucoup plus de parties dans votre code qui devraient (ou pourraient) être refactorisées.

Commençons par extraire les constantes. Mettez simplement chaque chaîne constante (comme "Deck 1 - Inside Twin") dans un const. Cela vous aidera si vous essayez de renommer l'un des ponts.

Ensuite, vous devriez respecter les conventions de codage les plus courantes. Les noms de classes en C# commencent toujours en majuscule (renommer room en Room).

Ensuite, vous pouvez extraire une méthode de

new ArrayList(); 
for (int i = 0; i < 30; i++) 
{ 
    groupC.Add(new room(3500, "C" + (i + 1))); 
} 

Alors, au lieu de votre instruction if vous pouvez utiliser un Dictionary ou quelque chose de similaire pour extraire la plate-forme correcte, ce qui serait tout simplement retirer le if-then-else.

probablement beaucoup plus, vous pouvez factoriser rendre votre code mieux :)