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

Question

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.

maybe you could create a factory method in the LibraryItem class. — vincendep
– vincendep, Commented Mar 21, 2021 at 14:15

azro · Accepted Answer · 2021-03-21 14:15:22Z

2

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

edited Mar 21, 2021 at 14:15

answered Mar 21, 2021 at 14:07

azro

54.2k9 gold badges39 silver badges75 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Franze Kane Over a year ago

God bless you, your solution is perfect, that's what I have been trying to do.

azro Over a year ago

@FranzeKane i've edit again, with a solution if you pass the Scanner to the object construtor

Franze Kane Over a year ago

That's even better, I wish there were more people like you!

Criss Hills · Accepted Answer · 2021-03-21 14:09:07Z

0

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

answered Mar 21, 2021 at 14:09

Criss Hills

1891 silver badge12 bronze badges

1 Comment

azro Over a year ago

That's exactly mine ;)

Anant Dahiya · Accepted Answer · 2021-03-21 14:13:39Z

0

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

answered Mar 21, 2021 at 14:13

Anant Dahiya

1

2 Comments

Franze Kane Over a year ago

You have only put storeItem(libraryItem);, but there are actually two, the other one is storeUser(libraryUser);, which is for else if (typeOfData.equals("Library User data"))

user13146129 Over a year ago

Welcome to SO. Read how to write a good answer? Explain your source code...

vincendep · Accepted Answer · 2021-03-21 14:35:39Z

0

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

edited Mar 21, 2021 at 14:35

answered Mar 21, 2021 at 14:26

vincendep

6325 silver badges10 bronze badges

2 Comments

user4910279 Over a year ago

You should call storeUser() instead of storeItem() for LibraryUser.

vincendep Over a year ago

I miss that too lol....neverttheless I would avoid to directly instatiate the subclasses of LibraryItem

Collectives™ on Stack Overflow

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

4 Answers 4

Simplify

Improve

3 Comments

1 Comment

2 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Simplify

Improve

3 Comments

1 Comment

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related