Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Java > Looking for a way to avoid use of instanceOf

Reply
Thread Tools

Looking for a way to avoid use of instanceOf

 
 
François Grondin
Guest
Posts: n/a
 
      12-04-2008
Hi everyone

I have the following problem. I'm developing a factory method that creates
objects from a list. This list contains objects that implements my interface
IRegulatedNME. Three classes implements this interface : Gate, Pump and
Weir. My factory method looks like this (I hope this code is clear enough) :

private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
{
List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();

for (IRegulatedNME regNME : aRegNMEList)
{
if (regNME.isGloballyCtrl())
{
ret.add(new GloballyCtrlConstraint(regNME);
}
else if (regNME.isLocallyCtrl())
{
ret.add(new LocallyCtrlConstraint(regNME);
}
else if (regNME.isUnctrl())
{
if (regNME instanceof Gate)
{
ret.add(new UnctrlGateConstraint((Gate) regNME);
}
else if (regNME instanceof Pump)
{
ret.add(new UnctrlPumpConstraint((Pump) regNME);
}
else // if (regNME instanceof Weir)
{
ret.add(new UnctrlWeirConstraint((Weir) regNME);
}
}
}

return ret;
}

As you could see, the cases "regNME.isGloballyCtrl()" or
"regNME.isLocallyCtrl()" apply to the abstract object. When
"regNME.isUnctrl()", then the constraints to be added depends on the type of
regNME (Gate, Pump or Weir). The list "aRegNMEList" may contain gates, pumps
or weirs at the same time. Is there a way to avoid the use of "instanceof"
in that situation? Could the generics be useful to solve this problem? Is
there a way to filter my list to extract only one type at a time (without
using "instanceof" of course)?

I googled "replace instanceof java" and "filter collection java" but was not
satisfied with the answers.

Thanks in advance for your replies.

François


 
Reply With Quote
 
 
 
 
Stefan Rybacki
Guest
Posts: n/a
 
      12-04-2008
"Fran��������������� ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿ ½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï¿½ï ¿½ï¿½ï¿½ï¿½" schrieb:
> Hi everyone
>
> I have the following problem. I'm developing a factory method that creates
> objects from a list. This list contains objects that implements my interface
> IRegulatedNME. Three classes implements this interface : Gate, Pump and
> Weir. My factory method looks like this (I hope this code is clear enough) :
>
> private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
> {
> List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();
>
> for (IRegulatedNME regNME : aRegNMEList)
> {
> if (regNME.isGloballyCtrl())
> {
> ret.add(new GloballyCtrlConstraint(regNME);
> }
> else if (regNME.isLocallyCtrl())
> {
> ret.add(new LocallyCtrlConstraint(regNME);
> }
> else if (regNME.isUnctrl())
> {
> if (regNME instanceof Gate)
> {
> ret.add(new UnctrlGateConstraint((Gate) regNME);
> }
> else if (regNME instanceof Pump)
> {
> ret.add(new UnctrlPumpConstraint((Pump) regNME);
> }
> else // if (regNME instanceof Weir)
> {
> ret.add(new UnctrlWeirConstraint((Weir) regNME);
> }
> }
> }
>
> return ret;
> }
>
> As you could see, the cases "regNME.isGloballyCtrl()" or
> "regNME.isLocallyCtrl()" apply to the abstract object. When
> "regNME.isUnctrl()", then the constraints to be added depends on the type of
> regNME (Gate, Pump or Weir). The list "aRegNMEList" may contain gates, pumps
> or weirs at the same time. Is there a way to avoid the use of "instanceof"
> in that situation? Could the generics be useful to solve this problem? Is
> there a way to filter my list to extract only one type at a time (without
> using "instanceof" of course)?
>
> I googled "replace instanceof java" and "filter collection java" but was not
> satisfied with the answers.


maybe the IRegulatedNME interface can implement a method called
AbstractConstraint getConstraint();
This way you can avoid instanceof and furthermore you can support any
AbstractConstraint without modifying/adding new instanceof clauses for each new
available constraint.
But I don't know whether it makes sense to let the regNME provide its constraints.

Regards
Stefan
>
> Thanks in advance for your replies.
>
> Fran�ois
>
>

 
Reply With Quote
 
 
 
 
François Grondin
Guest
Posts: n/a
 
      12-04-2008

"Stefan Rybacki" <(E-Mail Removed)> a écrit dans le message de news:
4937fd15$(E-Mail Removed)-rostock.de...
> "Fran????????????????????????????????????????????? ???????" schrieb:
>> Hi everyone
>>
>> I have the following problem. I'm developing a factory method that
>> creates objects from a list. This list contains objects that implements
>> my interface IRegulatedNME. Three classes implements this interface :
>> Gate, Pump and Weir. My factory method looks like this (I hope this code
>> is clear enough) :
>>
>> private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
>> {
>> List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();
>>
>> for (IRegulatedNME regNME : aRegNMEList)
>> {
>> if (regNME.isGloballyCtrl())
>> {
>> ret.add(new GloballyCtrlConstraint(regNME);
>> }
>> else if (regNME.isLocallyCtrl())
>> {
>> ret.add(new LocallyCtrlConstraint(regNME);
>> }
>> else if (regNME.isUnctrl())
>> {
>> if (regNME instanceof Gate)
>> {
>> ret.add(new UnctrlGateConstraint((Gate) regNME);
>> }
>> else if (regNME instanceof Pump)
>> {
>> ret.add(new UnctrlPumpConstraint((Pump) regNME);
>> }
>> else // if (regNME instanceof Weir)
>> {
>> ret.add(new UnctrlWeirConstraint((Weir) regNME);
>> }
>> }
>> }
>>
>> return ret;
>> }
>>
>> As you could see, the cases "regNME.isGloballyCtrl()" or
>> "regNME.isLocallyCtrl()" apply to the abstract object. When
>> "regNME.isUnctrl()", then the constraints to be added depends on the type
>> of regNME (Gate, Pump or Weir). The list "aRegNMEList" may contain gates,
>> pumps or weirs at the same time. Is there a way to avoid the use of
>> "instanceof" in that situation? Could the generics be useful to solve
>> this problem? Is there a way to filter my list to extract only one type
>> at a time (without using "instanceof" of course)?
>>
>> I googled "replace instanceof java" and "filter collection java" but was
>> not satisfied with the answers.

>
> maybe the IRegulatedNME interface can implement a method called
> AbstractConstraint getConstraint();
> This way you can avoid instanceof and furthermore you can support any
> AbstractConstraint without modifying/adding new instanceof clauses for
> each new
> available constraint.
> But I don't know whether it makes sense to let the regNME provide its
> constraints.
>
> Regards
> Stefan
>>
>> Thanks in advance for your replies.
>>
>> Fran?ois


Hi Stefan

Thanks for your reply. Effectively, I don't like the idea of adding a method
getConstraint() in IRegulatedNME.

I forgot to mention that I may construct more than one constraint at a time.
My example should be :

private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
{
List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();

for (IRegulatedNME regNME : aRegNMEList)
{
if (regNME.isGloballyCtrl())
{
ret.add(new GloballyCtrlConstraint(regNME);
ret.add(new AnotherGloballyCtrlConstraint(regNME);
}
else if (regNME.isLocallyCtrl())
{
ret.add(new LocallyCtrlConstraint(regNME);
}
else if (regNME.isUnctrl())
{
ret.add(new AnotherConstraint(regNME);

if (regNME instanceof Gate)
{
ret.add(new UnctrlGateConstraint((Gate) regNME);
ret.add(new AnotherUnctrlGateConstraint((Gate) regNME);
}
else if (regNME instanceof Pump)
{
ret.add(new UnctrlPumpConstraint((Pump) regNME);
}
else // if (regNME instanceof Weir)
{
ret.add(new UnctrlWeirConstraint((Weir) regNME);
}
}
}

return ret;
}

This is closer to what I really need.

Regards

Francois


 
Reply With Quote
 
Stefan Ram
Guest
Posts: n/a
 
      12-04-2008
"François Grondin" <francois.grondin@no_spam.bpr-cso.com> writes:
>ret.add(new UnctrlGateConstraint((Gate) regNME);


This is indecipherable because the parentheses do not match.

 
Reply With Quote
 
François Grondin
Guest
Posts: n/a
 
      12-04-2008

"Stefan Ram" <(E-Mail Removed)-berlin.de> a écrit dans le message de news:
http://www.velocityreviews.com/forums/(E-Mail Removed)-berlin.de...
> "François Grondin" <francois.grondin@no_spam.bpr-cso.com> writes:
>>ret.add(new UnctrlGateConstraint((Gate) regNME);

>
> This is indecipherable because the parentheses do not match.
>


OK. Here is a more complete and corrected version of the code.

private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
{
List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();

for (IRegulatedNME regNME : aRegNMEList)
{
if (regNME.isGloballyCtrl())
{
ret.add(new GloballyCtrlConstraint(regNME));
ret.add(new AnotherGloballyCtrlConstraint(regNME));
}
else if (regNME.isLocallyCtrl())
{
ret.add(new LocallyCtrlConstraint(regNME));
}
else if (regNME.isUnctrl())
{
ret.add(new AnotherConstraint(regNME));

if (regNME instanceof Gate)
{
ret.add(new UnctrlGateConstraint((Gate) regNME));
ret.add(new AnotherUnctrlGateConstraint((Gate) regNME));
}
else if (regNME instanceof Pump)
{
ret.add(new UnctrlPumpConstraint((Pump) regNME));
}
else // if (regNME instanceof Weir)
{
ret.add(new UnctrlWeirConstraint((Weir) regNME));
}
}
}

return ret;
}

I hope this is clearer this time. Sorry for the inconvenience.

François


 
Reply With Quote
 
Andreas Leitgeb
Guest
Posts: n/a
 
      12-04-2008
François Grondin <francois.grondin@no_spam.bpr-cso.com> wrote:
> Thanks for your reply. Effectively, I don't like the idea of adding a method
> getConstraint() in IRegulatedNME.
>
> I forgot to mention that I may construct more than one constraint at a time.
> My example should be :


how about getListOfConstraints() then?

> ret.add(new GloballyCtrlConstraint(regNME);
> ret.add(new AnotherGloballyCtrlConstraint(regNME);

for (BaseConstraint bc : regNME.getListOfConstraints() ) ret.add(bc);

If you cannot add it to regNME, then there is hardly any way to avoid
those various instanceof's.

Oh, there is another one: add an enumeration with all the regNME types
and do a switch on regNME.myType()

PS: please take the effort to trim the quotes.
 
Reply With Quote
 
Stefan Ram
Guest
Posts: n/a
 
      12-04-2008
"François Grondin" <francois.grondin@no_spam.bpr-cso.com> writes:
>if (regNME instanceof Gate)
>{
> ret.add(new UnctrlGateConstraint((Gate) regNME));
> ret.add(new AnotherUnctrlGateConstraint((Gate) regNME));
>}
>else if (regNME instanceof Pump)
>{
> ret.add(new UnctrlPumpConstraint((Pump) regNME));
>}
>else // if (regNME instanceof Weir)
>{
> ret.add(new UnctrlWeirConstraint((Weir) regNME));
>}


regNME.addTo( ret );

, where the implementation of »addTo« differs
for »Gate«, »Pump« and »Weir« as given above.

»This is the heart of why ObjectOrientedProgramming is better.«

http://c2.com/cgi/wiki?ReplaceCondit...thPolymorphism

 
Reply With Quote
 
Tom Anderson
Guest
Posts: n/a
 
      12-04-2008
On Thu, 4 Dec 2008, Stefan Ram wrote:

> "François Grondin" <francois.grondin@no_spam.bpr-cso.com> writes:
>> ret.add(new UnctrlGateConstraint((Gate) regNME);

>
> This is indecipherable because the parentheses do not match.


No, it's incorrect because the parentheses do not match, but it's trivial
to decipher. There's nowhere you could take an open paren out and have it
be correct, which means there's a close paren missing, and there's only
one place it would be syntactically permitted. It's also obvious what this
line is doing semantically, and thus where the missing paren belongs.

I have no problem with pointing out that people's code is wrong, but
please, let's keep a sense of perspective. We don't do anyone any favours
by getting preoccupied with minutiae like this.

tom

--
I'm angry, but not Milk and Cheese angry. -- Mike Froggatt
 
Reply With Quote
 
Tom Anderson
Guest
Posts: n/a
 
      12-04-2008
On Thu, 4 Dec 2008, Andreas Leitgeb wrote:

> François Grondin <francois.grondin@no_spam.bpr-cso.com> wrote:
>> Thanks for your reply. Effectively, I don't like the idea of adding a method
>> getConstraint() in IRegulatedNME.


One minor tiny niggle: in java, it's not conventional to stick an 'I' on
the front of an interface name. RegulatedNME is fine. And if you can find
a proper word that can be used instead of NME, that would be beautiful!

>> I forgot to mention that I may construct more than one constraint at a time.
>> My example should be :

>
> how about getListOfConstraints() then?


Yup. Personally, i'd call it getConstraints(), and have it return a Set,
unless there's an ordering on constraints. Or it could return a
Collection.

Francois [1], you might well not like this solution, but it's definitely
the Official Orthodox Object-Oriented solution. Not that i'm saying it's
the right solution - you might see constraints as being a separate concern
from whatever it is that RegulatedNMEs are mostly about.

>> ret.add(new GloballyCtrlConstraint(regNME);
>> ret.add(new AnotherGloballyCtrlConstraint(regNME);

> for (BaseConstraint bc : regNME.getListOfConstraints() ) ret.add(bc);
>
> If you cannot add it to regNME, then there is hardly any way to avoid
> those various instanceof's.
>
> Oh, there is another one: add an enumeration with all the regNME types
> and do a switch on regNME.myType()


That's not a bad idea.

While we're on the subject, i'd also use an enumeration for the control
state; rather than those three boolean isXXXCtrl(), methods, define:

enum Control {
GLOBAL,
LOCAL,
UNCONTROLLED
}

And have a method:

public Control getControl() ;

Then instead of the if-else-if cascade, do:

switch (regNME.getControl()) {
case GLOBAL:
// etc
case LOCAL:
// etc
case UNCONTROLLED:
// etc
}

Anyway, back to the question of replacing the instanceof. There is another
way: bounce dispatch. This is an arcane technique of the OO masters, which
i will now share with you. It goes by many names - i don't think it has a
standard one. Most people will know it from the Visitor pattern, but it's
not the same as Visitor - Visitor is this plus the Internal Iterator
pattern.

The key is an interface like this:

interface RegulatedNMEHandler {
public void handle(Gate gate) ;
public void handle(Pump pump) ;
public void handle(Weir weir) ;
}

You then add a method to RegulatedNME:

interface RegulatedNME {
public void accept(RegulatedNMEHandler hdlr) ;
}

With implementations that look like this:

class Gate implements RegulatedNME {
public void accept(RegulatedNMEHandler hdlr) {
hdlr.handle(this) ;
}
}

You now write your method like this:

private List<AbstractConstraint> create(List<RegulatedNME> aRegNMEList)
{
final List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();
for (RegulatedNME regNME : aRegNMEList)
{
if (regNME.isGloballyCtrl())
{
ret.add(new GloballyCtrlConstraint(regNME));
ret.add(new AnotherGloballyCtrlConstraint(regNME));
}
else if (regNME.isLocallyCtrl())
{
ret.add(new LocallyCtrlConstraint(regNME));
}
else if (regNME.isUnctrl())
{
ret.add(new AnotherConstraint(regNME));
regNME.accept(new RegulatedNMEHandler() {
public void handle(Gate gate) {
ret.add(new UnctrlGateConstraint(gate);
ret.add(new AnotherUnctrlGateConstraint(gate);
}
public void handle(Pump pump) {
ret.add(new UnctrlPumpConstraint(pump);
}
public void handle(Weir weir) {
ret.add(new UnctrlWeirConstraint(weir);
}
}) ;
}
}
return ret;
}

The idea is that you separate out the polymorphism bit and the bit where
you actually do something differently depending on class. None of the
stuff you add to RegulatedNME and its implementations is specific to
constraints - it's completely generic. You confine the code that's about
constraints to an implementation of the handler interface. If there are
other places in your app where you do a similar set of instanceof tests,
you can reuse the same dispatch mechanism there.

In this example, i've made the handler an anonymous local class, but
there's no need for it to be that way. It could be a named top-level
class.

Yet another way to do this would be with a trendy modern dynamic
codey-wodey sort of approach. Something like:

interface ConstraintFactory {
public void makeConstraints(RegulatedNME rnme, List<AbstractConstraint> constraints) ;
}

class GateConstraintFactory implements ConstraintFactory {
public void makeConstraints(RegulatedNME rnme, List<AbstractConstraint> constraints) {
Gate gate = (Gate)rnme ;
constraints.add(new UnctrlGateConstraint(gate)) ;
constraints.add(new AnotherUnctrlGateConstraint(gate)) ;
}
}

// likewise for Pump and Weir

class ConstraintMaker {
private Map<Class<RegulatedNME>, ConstraintFactory> factories ;
public ConstraintMaker() {
factories = new HashMap<Class<RegulatedNME>, ConstraintFactory>() ;
factories.put(Gate.class, new GateConstraintFactory()) ;
factories.put(Pump.class, new PumpConstraintFactory()) ;
factories.put(Weir.class, new WeirConstraintFactory()) ;
}
public List<AbstractConstraint> makeConstraints(RegulatedNME rnme) {
List<AbstractConstraint> constraints = new ArrayList<AbstractConstraint>() ;
if (rnme.isGloballyCtrl()) {
constraints.add((new GloballyCtrlConstraint(regNME)) ;
constraints.add(new AnotherGloballyCtrlConstraint(regNME)) ;
}
else if (regNME.isLocallyCtrl()) {
constraints.add(new LocallyCtrlConstraint(regNME)) ;
}
else if (regNME.isUnctrl()) {
constraints.add(new AnotherConstraint(regNME)) ;
ConstraintFactory factory = factories.get(rnme.getClass()) ;
factory.makeConstraints(rnme, constraints) ;
}
return constraints ;
}
}

The trouble with this is that if you introduce a new subclass of
RegulatedNME, it will still compile, but will fail at runtime. And it's
pretty verbose. In languages with first-class functions, it can be a lot
cleaner.

tom

[1] I can't type cedillas - please accept my apologies!

--
I'm angry, but not Milk and Cheese angry. -- Mike Froggatt
 
Reply With Quote
 
Daniel Pitts
Guest
Posts: n/a
 
      12-04-2008
François Grondin wrote:
> Hi everyone
>
> I have the following problem. I'm developing a factory method that creates
> objects from a list. This list contains objects that implements my interface
> IRegulatedNME. Three classes implements this interface : Gate, Pump and
> Weir. My factory method looks like this (I hope this code is clear enough) :
>
> private List<AbstractConstraint> create(List<IRegulatedNME> aRegNMEList)
> {
> List<AbstractConstraint> ret = new ArrayList<AbstractConstraint>();
>
> for (IRegulatedNME regNME : aRegNMEList)
> {
> if (regNME.isGloballyCtrl())
> {
> ret.add(new GloballyCtrlConstraint(regNME);
> }
> else if (regNME.isLocallyCtrl())
> {
> ret.add(new LocallyCtrlConstraint(regNME);
> }
> else if (regNME.isUnctrl())
> {
> if (regNME instanceof Gate)
> {
> ret.add(new UnctrlGateConstraint((Gate) regNME);
> }
> else if (regNME instanceof Pump)
> {
> ret.add(new UnctrlPumpConstraint((Pump) regNME);
> }
> else // if (regNME instanceof Weir)
> {
> ret.add(new UnctrlWeirConstraint((Weir) regNME);
> }
> }
> }
>
> return ret;
> }
>
> As you could see, the cases "regNME.isGloballyCtrl()" or
> "regNME.isLocallyCtrl()" apply to the abstract object. When
> "regNME.isUnctrl()", then the constraints to be added depends on the type of
> regNME (Gate, Pump or Weir). The list "aRegNMEList" may contain gates, pumps
> or weirs at the same time. Is there a way to avoid the use of "instanceof"
> in that situation? Could the generics be useful to solve this problem? Is
> there a way to filter my list to extract only one type at a time (without
> using "instanceof" of course)?
>
> I googled "replace instanceof java" and "filter collection java" but was not
> satisfied with the answers.
>
> Thanks in advance for your replies.
>
> François
>
>

This whole thing can be simplified:

ret.add(regNME.createConstraint());

This is what polymorphism is for

--
Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Looking for a way to avoid copy-n-pasting Pavel C++ 2 04-18-2009 06:07 AM
instanceof NOT (always) bad? The instanceof myth. dmx_dawg@hotmail.com Java 21 07-20-2006 07:06 PM
Why should use of instanceof be avoided HalcyonWild Java 30 05-27-2005 11:47 AM
valid use of instanceof or a better way? VisionSet Java 13 11-18-2003 10:08 PM
when to use instanceof? Digital Puer Java 3 09-05-2003 12:47 AM



Advertisments