Tell don't ask and model view seperation

[Don Kiddick]

Hi,

I find there is a tension between the principles of "tell don't ask" and model view seperation. For example, I always end up with code like this :

  	public Image getImage(Object element)
	{
		Image image = null;
		if (element instanceof IStructure)
		{
			image = SWTResourceManager.getImage(STRUCTURE_IMAGE);
		}
		else if (element instanceof IEquityProduct)
		{
			image = SWTResourceManager.getImage(INSTRUMENT_IMAGE);
		}
		else if (element instanceof CompositeProduct)
		{
			image = SWTResourceManager.getImage(COMPOSITE_INSTRUMENT_IMAGE);
		}
		else if (element instanceof IEquityTrade)
		{
			if (((IEquityTrade) element).isTradable())
			{
				image = SWTResourceManager.getImage(TRADE_INSTRUMENT_IMAGE);
			}
			else
			{
				image = SWTResourceManager.getImage(TRADE_INSTRUMENT_DISABLED_IMAGE);
			}
		}
		else if (element instanceof IEquityMarket)
		{
			image = SWTResourceManager.getImage(MARKET_INSTRUMENT_IMAGE);
		}
		return image;
	}  

Not ideal, switching on the objects type.
However to maintain model/view seperation, I cannot ask the object for it's image.
I could also create a map of class to image, but that is just another way of writing the switch statement, and it doesn't handle subclasses so well
I've tried using a double dispatch pattern, and while this does get rid of the switch, it makes the code much more confusing.

So in the end, I just live with the switch statement. Are there any other patterns/approaches I could use here?

thanks, Don.

[Hong Anderson]

Conditional code is fine. What make you think that it is a problem?

[Jelle Klap]

You could simulate a double dispatch approach if you really want to, but it makes the code much more complex and it doesn't do much to improve maintainability either:

package nl.jekl.javaranch;

public class DoubleDispatchExample {
	public static void main(String[] args) {
		Element elementA = new ElementTypeA();
		ReturnValue resultA = elementA.accept(new ElementVisitorImpl());
		System.out.println(resultA);
		
		Element elementB = new ElementTypeB();
		ReturnValue resultB = elementB.accept(new ElementVisitorImpl());
		System.out.println(resultB);
		
		Element elementC = new ElementTypeC();
		ReturnValue resultC = elementC.accept(new ElementVisitorImpl());
		System.out.println(resultC);		
	}
}


interface ElementVisitor {
	ReturnValue visit(ElementTypeA element);

	ReturnValue visit(ElementTypeB element);

	ReturnValue visit(ElementTypeC element);
}


interface Element {
	ReturnValue accept(ElementVisitor visitor);
}


interface ReturnValue {
}


class ElementVisitorImpl implements ElementVisitor {

	public ReturnValue visit(ElementTypeA element) {
		return new ReturnValueTypeA();
	}

	public ReturnValue visit(ElementTypeB element) {
		return new ReturnValueTypeB();
	}

	public ReturnValue visit(ElementTypeC element) {
		return new ReturnValueTypeC();
	}
}


class ElementTypeA implements Element {
	public ReturnValue accept(ElementVisitor visitor) {
		return visitor.visit(this);
	}
}


class ElementTypeB implements Element {
	public ReturnValue accept(ElementVisitor visitor) {
		return visitor.visit(this);
	}
}


class ElementTypeC implements Element {
	public ReturnValue accept(ElementVisitor visitor) {
		return visitor.visit(this);
	}
}


class ReturnValueTypeA implements ReturnValue {
	
	@Override
	public String toString(){
		return "Type A";
	}
}


class ReturnValueTypeB implements ReturnValue {
	
	@Override
	public String toString(){
		return "Type B";
	}
}


class ReturnValueTypeC implements ReturnValue {
	
	@Override
	public String toString(){
		return "Type C";
	}
}

So exactly how far are you willing to take this to avoid those instanceof checks? ;)

Edit: Yeah, so I sort of skimmed over the part of your OP where it says you already had a look at the DD approach, and shot it down for the same reason I mentioned. I'll stop wasting everybody's time now...

[Don Kiddick]

Exactly. So just live with conditional is probably the best bet. Ta.

[David Newton]

It's not too much fun in Java, but I've often used decorator-like things when stuff like this pops up.

[Ilja Preuss]

The only problem I see with the conditionals is that this way the method is tightly coupled to all possibly existing types. At some point, you might want to separate some of those types into different, potentially optional, moduls. Then it really starts to make sense to use some kind of registry, like a HashMap - one, that doesn't get filled from a central place, but that different moduls/plugins contribute entries to.