CODEWITHSUNDEEP
javaif-statementrefactoring
Franze Kane
Franze Kane

Reputation: 3

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

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.

Upvotes: 0

Views: 2695

Answers (4)

vincendep
vincendep

Reputation: 607

maybe you could use a factory method in the LibraryItem class.

class LibraryItem {

    public static LibraryItem from(String typeOfData) {
        if (typeOfData.equals("Book data")) {
            return new Book();
        }
        if (typeOfData.equals("Periodical data")) {
            return new Periodical();
        }
        if (typeOfData.equals("CD data")) {
            return new CD();
        }
        if (typeOfData.equals("DVD data")) {
            return new DVD();
        }
        if (typeOfData.equals("Library User data")) {
            return new LibraryUser();
        } 
        
        throw new IllegalArgumentException();
    }
}

and then

System.out.println(lineOfText);   
Scanner scanner2 = new Scanner(lineOfText); 
LibraryItem libraryItem = LibraryItem.from(typeOfData);
libraryItem.readData(scanner2);
storeItem(libraryItem);
scanner2.close(); // ends scanner2

EDIT

I ve just seen that probably LibraryUser do not extends LibraryItem. But maybe you could extract an interface for the method readData(Scanner s) and apply the same pattern

Upvotes: 0

azro
azro

Reputation: 54168

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

Upvotes: 2

Anant Dahiya
Anant Dahiya

Reputation: 1

System.out.println(lineOfText);   
Scanner scanner2 = new Scanner(lineOfText);
LibraryItem libraryItem = null;
if (typeOfData.equals("Book data")) 
{
   LibraryItem libraryItem = new Book();                   
}
else if (typeOfData.equals("Periodical data"))
{
   LibraryItem libraryItem = new Periodical();
}
else if (typeOfData.equals("CD data"))
{
   LibraryItem libraryItem = new CD();                     
} 
else if (typeOfData.equals("DVD data"))
{
   LibraryItem libraryItem = new DVD();
}
else if (typeOfData.equals("Library User data"))
{
   LibraryUser libraryUser = new LibraryUser();
}
if(libraryItem != null){
    libraryItem.readData(scanner2);
    storeItem(libraryItem);
}

scanner2.close();

Upvotes: 0

Criss Hills
Criss Hills

Reputation: 189

This is the closest i could get:

        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

Upvotes: 0

Related Questions