This week's book giveaway is in the Other Languages forum.
We're giving away four copies of Functional Reactive Programming and have Stephen Blackheath and Anthony Jones on-line!
See this thread for details.
Win a copy of Functional Reactive Programming this week in the Other Languages forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Code smell - if statements and instanceof

 
Pho Tek
Ranch Hand
Posts: 782
Chrome Python Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a class that looks like this .

A typical execution:

Currently the do method performs a bunch of if branching based on the value in Holder.clazz:

How can I improve this code ?

Thanks

P
 
Ilja Preuss
author
Sheriff
Posts: 14112
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So, if I understand you correctly, you want to do different things based on the type of an object, but you don't want to implement those things inside the object itself?

Sounds like a job for the Visitor pattern...
 
Lasse Koskela
author
Sheriff
Posts: 11962
5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would also consider introducing a factory class for creating different implementations of the Holder class based on the Class you pass to the factory method. Ilja's tip about the Visitor pattern sounds good as well.
 
Joe Nguyen
Ranch Hand
Posts: 161
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What is the purpose of supplying both the object and its class?

new Holder(po, PurchaseOrder.class);
 
Junilu Lacar
Bartender
Pie
Posts: 7778
62
Android Eclipse IDE IntelliJ IDE Java Linux Mac Scala Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
My bet ($0.02) is that the code in each of the if blocks smells of feature envy:

 
steve souza
Ranch Hand
Posts: 862
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The code definitely has a smell. Pretty much any time you have an if condition based on type it should be refactored unless it is in a factory method.

It is a little hard to give a definitive recommendation without knowing a little more about what you are trying to do. In your example the Holder has no value if it just executes the other classes do method.

I would make all 3 classes implement a common interface (I'll call it the DoInterface). Forgive any syntax errors.




Your code would look as follows now:




[ edited to break long line -ds ]
[ August 21, 2004: Message edited by: Dirk Schreckmann ]
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic