“if” statement vs OO Design - 2

I encountered similar problem “if” statement vs OO Design - 1 but it is slightly different. Here is the problem that open the popup (different objects/popups) onValueChange of listbox

Popup1 p1; // different objects
Popup2 p2; // different objects
Popup3 p3;
...

listbox.add("p1");
listbox.add("p2");
listbox.add("p3");
...

listbox.addChan开发者_开发百科geHandler() {
    if(getSelectedItem().equals("p1")){
       p1 = new Popup1();
       p1.show();
    } else if() {...}
      ....
}

I don't want to write "if" that if p1 then p1 = new Popup1(); p1.center();

How I can handle this situation? Any design-pattern?

Here is my solution but it is so costly

map() {

    map.put("p1", new Popup1());
    map.put("p2", new Popup2());
    map.put("p3", new Popup3()); 
}

onValueChange() {
    map.get(selectedItem).show();
}

One drawback is initialization all the popups. but it is require only when valueChange

---

Well, if P1 and P2 both implement the same interface, i.e

Popup1 implements Showable
Popup2 implements Showable

then you could simply do

Showable showable = (Showable) listbox.getSelected();
showable.show();

---

I agree with leveraging the common base class when you can, but you can't always add a method to the base class for every usage that might call for selecting between different subclasses.

Your "map" solution is a decent approach for cases where the selection logic is specific to a piece of code, like matching user action to an object (e.g. popup), and you can't find a way to leverage the common base class.

One drawback is initialization all the popups. but it is require only when valueChange

You should defer the instantiation until you need it:

interface Showable {
    void show();
}
map() {

    map.put("p1", new Showable() { void show() { new Popup1().show(); } } );
    map.put("p2", new Showable() { void show() { new Popup2().show(); } } );
    map.put("p3", new Showable() { void show() { new Popup3().show(); } } ); 
}

onValueChange() {
    map.get(selectedItem).show();
}

The anonymous classes are stateless, so if you wanted to be extra efficient, you could create the anonymous instances once and reuse them.

---

Yes, this design pattern is called polymorphism. You define a super class for all your PopupX classes. In this super class you define a method "onChange". Then you realize this method in each subclass. When you select an item, you define a reference on the superclass object and call "onChange".

---

If there really are three different classes Popup1, Popup2, Popup3, then they should either inherit from a common base class (presumably called Popup) or implement a common interface (as already suggested). But it appears you need a way to map strings identifying popups to popup objects. Sounds like an opportunity for factory pattern, which would look something like this.

public class PopupFactory {
   private PopupFactory() { }
   public static Popup createPopup(String popupIdentifier) {
      if ("p1".equals(popupIdentifier)) return new Popup1();
      else if ("p2".equals(popupIdentifier)) return new Popup2();
      ...
   }

And then would be used like:

listbox.addChangeHandler() {
   Popup popup = PopupFactory.createPopup(getSelectedItem());
   popup.show();
   ....
}

---

Your "costly" solution seems just fine to me. Better yet, from your map you can populate the listbox, thus:

for (String key: map.keyset())
    listbox.add(key);

Of course you may want the keys in a particular order, in which case first add them to a sorted list.

ArrayList<String> list = new ArrayList<String>();
list.addAll(map.keySet());
Collections.sort(list);
for (String key: list)
    listbox.add(key);

---

Unfortunately there's no way around what you mentioned given the constraints.

If the popups are only slightly different they could be implemented as 1 class with a property that defines what type of popup you have.

Otherwise the best you can do it encapsulate it in a factory class like @JST mentioned, that way the ugly logic exists in just one place. I'm not sure why you consider your map solution costly though.

If you want to dynamically invoke a constructor based on name, you're going to have to look at a dynamic language instead of Java.