CODEWITHSUNDEEP
javaif-statementswitch-statement
Gad Wissberg
Gad Wissberg

Reputation: 475

Replace elseif with a more generic way

I have this piece of code:

@Override
public void inform(String data) {
    if (data.equals(C.SubscriptionEvents.WINDOW_CLOSED)) {
        File tempFolder = new File("temp");
        File[] files = tempFolder.listFiles();
        if (files != null) {
            for (File f : files) f.delete();
        }
    } else if (data.equals(C.Controller.Commands.SELECT_MODE_VERTICES)) {
        MainModel.setCurrentMode(Mode.VERTICES);
        display.getInfoSection().repaint();
    } else if (data.equals(C.Controller.Commands.SELECT_MODE_LINES)) {
        MainModel.setCurrentMode(Mode.LINES);
        display.getInfoSection().repaint();
    } else if (data.equals(C.Controller.Commands.SELECT_MODE_SECTORS)) {
        MainModel.setCurrentMode(Mode.SECTORS);
        display.getInfoSection().repaint();
    }
}

The method gets a string which is a Command name. According to the name, it does a specified behavior. As you can see, it starts to have too much elseifs (and probably will have more). This method belongs to an interface which is shared between packages so I decided to make the parameter as string. Is there a better way to do it to avoid the method to be huge when there will be lots of commands (this includes switch case too)?

Upvotes: 2

Views: 120

Answers (4)

ZRab
ZRab

Reputation: 26

You could simplify it this way...

@Override
public void inform(String data) {
    Map<String, String> map = new HashMap<String, int>();
    map.put(C.Controller.Commands.SELECT_MODE_VERTICES, Mode.VERTICES);
    map.put(C.Controller.Commands.SELECT_MODE_LINES), Mode.LINES);
    map.put(C.Controller.Commands.SELECT_MODE_SECTORS, Mode.SECTORS);
    if (data.equals(C.SubscriptionEvents.WINDOW_CLOSED)) {
        File tempFolder = new File("temp");
        File[] files = tempFolder.listFiles();
        if (files != null) {
            for (File f : files) f.delete();
        }
    } else if (map.containsKey(data)) {
        MainModel.setCurrentMode(map.get(key));
        display.getInfoSection().repaint();
    } 
}

Upvotes: 0

Oleg Cherednik
Oleg Cherednik

Reputation: 18245

What about to use Enum instead of if...else?:

enum Event {
    NULL(null, context -> { }),
    WINDOWS_CLOSE(C.SubscriptionEvents.WINDOW_CLOSED, context -> {
        File tempFolder = new File("temp");
        File[] files = tempFolder.listFiles();
        if (files != null) {
            for (File f : files) f.delete();
        }
    }),
    SELECT_MODE_VERTICES(C.Controller.Commands.SELECT_MODE_VERTICES, context -> {
        MainModel.setCurrentMode(Mode.VERTICES);
        display.getInfoSection().repaint();
    }),
    SELECT_MODE_LINES(C.Controller.Commands.SELECT_MODE_VERTICES, context -> {
        MainModel.setCurrentMode(Mode.LINES);
        display.getInfoSection().repaint();
    }),
    SELECT_MODE_SECTORS(C.Controller.Commands.SELECT_MODE_SECTORS, context -> {
        MainModel.setCurrentMode(Mode.SECTORS);
        display.getInfoSection().repaint();
    });

    private final String id;
    private final Consumer<Foo> consumer;

    Event(String id, Consumer<Foo> consumer) {
        this.id = id;
        this.consumer = consumer;
    }

    public final void accept(Foo context) {
        consumer.accept(context);
    }

    public static Event selectEvent(String data) {
        for (Event event : values())
            if (event.id.equals(data))
                return event;
        return NULL;
    }
}

And your code will be look like this one:

Event.selectEvent(data).accept(this);

Upvotes: 0

Tom Drake
Tom Drake

Reputation: 537

You could use an enum class, as follows:

public enum Command {
    WINDOW_CLOSED { //C.SubscriptionEvents.WINDOW_CLOSED
        public void invoke() {
            File tempFolder = new File("temp");
            File[] files = tempFolder.listFiles();
            if (files != null) {
                for (File f : files) f.delete();
            }
        }
    }
    ,SELECT_MODE_VERTICES { // C.Controller.Commands.SELECT_MODE_VERTICES
        public void invoke() {
            MainModel.setCurrentMode(Mode.VERTICES);
            display.getInfoSection().repaint();
        }
    }
    ,SELECT_MODE_LINES { // C.Controller.Commands.SELECT_MODE_LINES
        public void invoke() {
            MainModel.setCurrentMode(Mode.LINES);
            display.getInfoSection().repaint();
        }
    }
    ,SELECT_MODE_SECTORS { // C.Controller.Commands.SELECT_MODE_SECTORS
        public void invoke() {
            MainModel.setCurrentMode(Mode.SECTORS);
            display.getInfoSection().repaint();
        }
    }
    ;
    public abstract void invoke();
}

Then replace the guts of your function above with this:

@Override
public void inform(String data) {
    Command.valueOf(data).invoke();
}

The names of your 'enum' values must exactly match the string values of the various things you are testing for in your original code (e.g. C.SubscriptionEvents.WINDOW_CLOSED, C.SubscriptionEvents.WINDOW_CLOSED)

Upvotes: 0

Ivan
Ivan

Reputation: 8758

You can check Command pattern https://www.baeldung.com/java-command-pattern but it may require quite extensive refactoring and make method inform() accept objects of type Command

Upvotes: 3

Related Questions