Mike Ross Mike Ross - 2 months ago 12
C# Question

How to make this c# code small or refactor this code

I am new with coding and I recently read article about code refactoring. So I have made a console application for booking rooms in a ship. I think in my project there are only 2 part where I need refactoring, which are as follows.
One is if else statement.

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);
}


and the other one is if else statement as follow

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");


What I don't understand is that my parameters are changing in both logic.
So how can I make a separate method for this logic when my cabin class is changing every time?

Answer Source

All your iterations can be joined together, therefore executed in one run like:

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);   

and the bunch of mutually exclusive ifs can be joined in a cse lik:

    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;              
}