12

I can't figure out a better solution to my problem. I have a view controller that presents a list of elements. Those elements are models that can be an instance of B, C, D, etc and inherit from A. So in that view controller, each item should go to a different screen of the application and pass some data when the user select one of them. The two alternatives that comes to my mind are (please ignore the syntax, it is not a specific language)

1) switch (I know that sucks)

//inside the view controller
void onClickItem(int index) {
    A a = items.get(index);

    switch(a.type) {
         case b:
             B b = (B)a;
             go to screen X;
             x.v1 = b.v1; // fill X with b data
             x.v2 = b.v2; 
         case c:
             go to screen Y;
         etc...
    }
}

2) polymorphism

//inside the view controller
void onClickItem(int index) {
    A a = items.get(index);
    Screen s = new (a.getDestinationScreen()); //ignore the syntax
    s.v1 = a.v1;   // fill s with information about A
    s.v2 = a.v2;
    show(s);
}

//inside B
Class getDestinationScreen(void) {
    return Class(X);
}

//inside C
Class getDestinationScreen(void) {
    return Class(Y);
}

My problem with solution 2 is that since B, C, D, etc are models, they shouldn't know about view related stuff. Or should they in that case?

4 Answers 4

6

I think that perhaps an implementation of the visitor pattern would be useful here. The B, C, and D class would need to be "visited" to determine the view type, but would not need to know anything about views. The ViewFactory (below) would visit the item and use polymorphism to determine the correct view to build. No switch statements. No asking about model internals to decide what to build. The visitor interface uses polymorphism to select the correct setter for the view. The setter can pass the item to the constructor of the specific view type (X or Y or Z) and that view can then populate its fields from the item.

   //inside the view controller
   void onClickItem(int index) {
      ViewFactoryVisitable a = items.get(index);
      ViewFactory aViewFactory = new ViewFactory(
      s = aViewFactory.getViewFor(a);
      show(s);
   }

--------

//Element interface
public interface ViewFactoryVisitable
{
    public void accept(ViewFactory theViewFactory);
}

---------

public interface ViewFactoryVisitor
{
   // one for each concrete type, polymorphism will choose correct setter
   public set setViewFor(B b);
   public set setViewFor(C c);
   public set setViewFor(D d);
}

--------

// B, C, D must implement this visitable interface
class B implements ViewFactoryVisitable
{ 
   ...

   //accept the ViewFactory as a visitor
   public void accept(ViewFactoryVisitor theViewFactoryVisitor)
   {
      theViewFactoryVisitor. setViewFor(this);
   }

   ...
} 

--------

class ViewFactory implements ViewFactoryVisitor
{
   ViewFactory(ViewFactoryVisitable theItem) {
      theItem.accept(this);
   }

   private View mView = null;
   ...

   public void setViewFor(B b) {
      // construct a view x and populate with data from b
      mView = new ViewX(b); 
   }

   public void setViewFor(C c) {
      mView = new ViewY(c); 
   }

   public void setViewFor(D d) {
      mView = new ViewZ(d); 
   }

   View getView() {
      return mView;
   }

} 
2
  • 1
    Should the implementation of accept not be "theViewFactoryVisitor.setViewFor(this);" Sorry if i'm being stupid!
    – Ryan
    Nov 9, 2016 at 14:06
  • @Ryan Good catch. This mistake has been here for 3 years! Nov 9, 2016 at 17:19
1

More of a comment than an answer, but I think it's a toss up. Either the View has to know all about the Model so it can choose the screen (switch) or the Model has to know all about the View so it can choose the screen (polymorphism). I think you have to choose what you think will be the simplest over time; there's no right answer to the question. (I hope someone can prove me wrong.) I do lean towards polymorphism, myself.

I bump into this problem a bit. The most annoying case was a Wanderer class, instances of which wandered about a map. To draw it, either the display needed to know about Wanderer or Wanderer needed to know about the display. The problem was there were two displays (with more coming along). As the number of different Wanderer subclasses was large and growing, I put the drawing code in the Wanderer subclasses. That meant each large class had exactly one method that needed to know about Graphics2D and exactly one method that needed to know about Java3D. Ugly.

I did end up splitting the class, giving me two parallel class structures. The Wanderer class was freed from knowing about the graphics, but the DrawWanderer class still needed to know more about Wanderer than was decent and it needed to know about two (and maybe more) completely different graphics environments (Views). (I suppose this splitting-the-class idea might be an answer of sorts, but all it really does is contain the problem a bit.)

I do think this is a very general and fundamental problem of Object Oriented design.

0

I think going with the switch is a better option than going with polymorphism for this case.

Its a fairly simple thing to do, so I don't think it has to be overcomplicated by use of polymorphism.

I'd like to coin in this blog post. Switch statements aren't necessarily ugly as long as you use them properly. And in your case, Abstracting models like that for use in a controller may be overkill and might bring forth unwanted results. Like violating the SRP.

2
  • I see your point. Well, I don't think polymorphism are overcomplicated. And the A class in my case is not abstract, it is in fact used. Thank you for your thoughts, although I'm still waiting for a better solution and more inclined to the polymorphism approach. Oct 23, 2013 at 13:06
  • 1
    no worries, just giving my 2 cents on the matter. Although to solve your problem with having to put view logic into your models, you can always just wrap them with decorators so your models stay free of view logic. Then you can use polymorphism on the decorator classes instead of the model.
    – Maru
    Oct 23, 2013 at 13:29
0

My problem with solution 2 is that since B, C, D, etc are models, they shouldn't know about view related stuff.

I agree with this concern. I also am a little concerned that objects that sit in a combobox would have behavior. I'm not sure that's a "bad thing" having never done it, it just seems like an unnatural choice to me.

Also, it does not seems like A and it's subclasses are the type that you have interesting polymorphism with. The interesting type is actually Screen. In this example, A is just a class that holds information to inform Screen creation.

If you make the combobox contain a list of whatever a.type returns then a switch statement seems more natural. However, instead of putting it right in the click event handler, I would put it in a ScreenFactory. Then you have:

//inside the view controller
void onClickItem(int index) {
    A a = items.get(index);

    s = _screenFactory.GetScreen(a);
    show(s);
    }
}

//inside a ScreenFactory implementation
internal Screen GetScreen(A typeIndicator)
{
switch(a.type) {
     case b:
         return new ScreenX();
     case c:
         return new ScreenY();
     etc...        
}

This lets you test the screen building behavior, and nicely pulls some functionality out of your UI. It keeps your View layering intact. Perhaps it simplifies your design, if it means A and subclasses can be collapsed into the type flag that they contain.

1
  • Thank you for the response tallseth. Unfortunately, the models contains a lot of information, not just their type or the destination view controller. Also, although I didn't mention, the ScreenX, ScreenY etc. need to receive information about B, C, D on construction, so I cannot use a factory only passing the type, I need to pass the model itself. Oct 23, 2013 at 13:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.