• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Code smell - if statements and instanceof

 
Ranch Hand
Posts: 782
Python Chrome Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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
 
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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...
 
author
Posts: 11962
5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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.
 
Ranch Hand
Posts: 161
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
What is the purpose of supplying both the object and its class?

new Holder(po, PurchaseOrder.class);
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
My bet ($0.02) is that the code in each of the if blocks smells of feature envy:

 
Ranch Hand
Posts: 862
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • 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 ]
 
Think of how dumb the average person is. Mathematically, half of them are EVEN DUMBER. Smart tiny ad:
a bit of art, as a gift, that will fit in a stocking
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic