How do I refactor multiple if-else statements in Java?

Tags: , ,



How do I refactor all this code that seems repetitive and too long, is there a way to make it shorter?

if (typeOfData.equals("Book data")) 
{
   System.out.println(lineOfText);   
   Scanner scanner2 = new Scanner(lineOfText); 
   LibraryItem libraryItem = new Book();
   libraryItem.readData(scanner2);
   storeItem(libraryItem);
   scanner2.close(); // ends scanner2                        
}
else if (typeOfData.equals("Periodical data"))
{
   System.out.println(lineOfText);                  
   Scanner scanner2 = new Scanner(lineOfText); 
   LibraryItem libraryItem = new Periodical(); // LibrayItem => Periodical(subtype)
   libraryItem.readData(scanner2);
   storeItem(libraryItem);
   scanner2.close(); // ends scanner2 
}
else if (typeOfData.equals("CD data"))
{
   System.out.println(lineOfText);                  
   Scanner scanner2 = new Scanner(lineOfText); 
   LibraryItem libraryItem = new CD(); // LibrayItem => CD(subtype)
   libraryItem.readData(scanner2);
   storeItem(libraryItem);
   scanner2.close(); // ends scanner2                     
} 
else if (typeOfData.equals("DVD data"))
{
   System.out.println(lineOfText);                  
   Scanner scanner2 = new Scanner(lineOfText); 
   LibraryItem libraryItem = new DVD();
   libraryItem.readData(scanner2);
   storeItem(libraryItem);
   scanner2.close(); // ends scanner2 
}
else if (typeOfData.equals("Library User data"))
{
   System.out.println(lineOfText);
   Scanner scanner2 = new Scanner(lineOfText);
   LibraryUser libraryUser = new LibraryUser();
   libraryUser.readData(scanner2);
   storeUser(libraryUser);
   scanner2.close(); // ends scanner2 
}

I have tried using the Switch statement but that does not work in this circumstance.

the “typeOfData” variable holds a String that is used to match relevant lines.

Answer

Simplify

You can extract the common lines, before or after the ifs

System.out.println(lineOfText);
Scanner scanner2 = new Scanner(lineOfText);

if (typeOfData.equals("Book data")) {
    LibraryItem libraryItem = new Book();
    libraryItem.readData(scanner2);
    storeItem(libraryItem);
} else if (typeOfData.equals("Periodical data")) {
    LibraryItem libraryItem = new Periodical(); // LibrayItem => Periodical(subtype)
    libraryItem.readData(scanner2);
    storeItem(libraryItem);
} else if (typeOfData.equals("CD data")) {
    LibraryItem libraryItem = new CD(); // LibrayItem => CD(subtype)
    libraryItem.readData(scanner2);
    storeItem(libraryItem);
} else if (typeOfData.equals("DVD data")) {
    LibraryItem libraryItem = new DVD();
    libraryItem.readData(scanner2);
    storeItem(libraryItem);
} else if (typeOfData.equals("Library User data")) {
    LibraryUser libraryUser = new LibraryUser();
    libraryUser.readData(scanner2);
    storeUser(libraryUser);
}

scanner2.close(); // ends scanner2 

Improve

You could imagine the constructors to take the Scanner as parameter like

public Book(Scanner sc) {
    readData(sc);
}

Then the ifs becomes

if (typeOfData.equals("Book data")) {
    storeItem(new Book(scanner2));
} else if (typeOfData.equals("Periodical data")) {
    storeItem(new Periodical(scanner2));
} else if (typeOfData.equals("CD data")) {
    storeItem(new CD(scanner2));
} else if (typeOfData.equals("DVD data")) {
    storeItem(new DVD(scanner2));
} else if (typeOfData.equals("Library User data")) {
    storeUser(new LibraryUser(scanner2));
}

Or a switch

switch (typeOfData) {
    case "Book data"            -> storeItem(new Book(scanner2));
    case "Periodical data"      -> storeItem(new Periodical(scanner2));
    case "CD data"              -> storeItem(new CD(scanner2));
    case "DVD data"             -> storeItem(new DVD(scanner2));
    case "Library User data"    -> storeUser(new LibraryUser(scanner2));
}


Source: stackoverflow