Best way to items in a JComboBox

432 Views Asked by At

I have a JComboBox with many items. I added an item listener to this combo box which stores the selected item:

    comboBox.addItemListener(new ItemListener() {

        @Override
        public void itemStateChanged(ItemEvent e) {
            option = (String) e.getItem();
        }
    });

I have a button and when it is clicked, the program carries out the task on the basis of that selection.

Button:

public void actionPerformed(ActionEvent e) {
  if (option.toLowerCase().compareTo("contrast stretching") == 0) { /* do sth */ } 
  else if (option.toLowerCase().compareTo("mean") == 0){ /* do sth else */ }
  // And many other else if statements

The actionPerformed function is way too long. What is the best way to write the code? I don't want to make single function too long.

3

There are 3 best solutions below

15
On BEST ANSWER

You can create an interface (e.g. Task) representing the task that needs to be executed. Then create an implementation of this interface for each of the values in the combo box (ContrastTask, MeanTask, ...). Finally, in your actionPerformed, write a factory method returning the correct implementation - it will be basically a "switch" returning the correct instance of Task. Then you can just run this task...

Task might look like this:

public interface Task {
  [whatever result it should return] execute([whatever parameters it needs]);
}

On of the implementations might look like this:

public class MeanTask implements Task {
  public int execute(int a, int b) {
    return (a + b) / 2;
  }
}

Your factory will look like this:

private Task create(String option) {
  if (option.toLowerCase().compareTo("mean") == 0) {
    return new MeanTask();
  }
  else if ....
}

The advantage of this is that the logic of each task is nicely encapsulated in its own class.

1
On

As vektor and Eugene posted I would combine both solutions to something like this:

public interface Task {
  [whatever result it should return] execute([whatever parameters it needs]);
}

//call this method in your constructor
private void initTasks() {
    this.tasks.put("option1", new Task() {
                public int execute(int a, int b) {
                   return (a + b) / 2;
                }
          });
    //init more tasks
    //this.task.put(..., ...);
}

protected Task getTask(String option) {
     return this.task(option);
}   
5
On

This can help you:

public class Main {
  private Map<String, BaseStrategy> strategyMap = new HashMap<String, BaseStrategy>();

  {
    strategyMap.put("case1", new Strategy1());
    strategyMap.put("case2", new Strategy2());
    strategyMap.put("case3", new Strategy3());
  }

  //...

  public void actionPerformed(ActionEvent e) {
    strategyMap.get(option.toLowerCase()).processEvent();
  }

  abstract class BaseStrategy {

    public abstract void processEvent();

  }

  class Strategy1 extends BaseStrategy {

    @Override
    public void processEvent() {
      //case 1
    }
  }

  class Strategy2 extends BaseStrategy {

    @Override
    public void processEvent() {
      //case 2
    }
  }

  class Strategy3 extends BaseStrategy {

    @Override
    public void processEvent() {
      //case 3
    }
  }
}

You can make a map where the key is a string that defines your command and the value is a strategy that processes your event. Now you can place your processing code even to other java files and process event with only 1 line of code.


Actually if you have too many cases - Map is a way better than if-else-if-... Map will find a proper strategy a way faster - O(ln n) instead of O(n).