Looking for a way to avoid use of instanceOf

Discussion in 'Java' started by François Grondin, Dec 4, 2008.

  1. 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
     
    François Grondin, Dec 4, 2008
    #1
    1. Advertising

  2. "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
    >
    >
     
    Stefan Rybacki, Dec 4, 2008
    #2
    1. Advertising

  3. "Stefan Rybacki" <> a écrit dans le message de news:
    4937fd15$-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
     
    François Grondin, Dec 4, 2008
    #3
  4. François Grondin

    Stefan Ram Guest

    "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.
     
    Stefan Ram, Dec 4, 2008
    #4
  5. "Stefan Ram" <-berlin.de> a écrit dans le message de news:
    -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
     
    François Grondin, Dec 4, 2008
    #5
  6. 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.
     
    Andreas Leitgeb, Dec 4, 2008
    #6
  7. François Grondin

    Stefan Ram Guest

    "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?ReplaceConditionalWithPolymorphism
     
    Stefan Ram, Dec 4, 2008
    #7
  8. François Grondin

    Tom Anderson Guest

    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
     
    Tom Anderson, Dec 4, 2008
    #8
  9. François Grondin

    Tom Anderson Guest

    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
     
    Tom Anderson, Dec 4, 2008
    #9
  10. François Grondin

    Daniel Pitts Guest

    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/>
     
    Daniel Pitts, Dec 4, 2008
    #10
  11. François Grondin

    Daniel Pitts Guest

    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.
    >>
    >> 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);

    or ret.addAll(regNME.getListOfConstraints());
    >
    > 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.



    --
    Daniel Pitts' Tech Blog: <http://virtualinfinity.net/wordpress/>
     
    Daniel Pitts, Dec 4, 2008
    #11
  12. Tom Anderson schrieb:
    > 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.


    I don't really like this approach because it means you have to adapt the
    interface whenever you add a new Constraint type which in return means you need
    to adjust all implemented to that changes.

    >
    > 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.


    I like the idea of using factories and a global registry though, because that is
    what we use over here in some cases as well and it is powerful, easy to maintain
    und to extend you can even drive the registry using reflection or by external
    files (factory.xml -> pointing to a factory to register) that can be scanned on
    startup.

    >
    > tom
    >
    > [1] I can't type cedillas - please accept my apologies!
    >
     
    Stefan Rybacki, Dec 4, 2008
    #12
  13. -----BEGIN PGP SIGNED MESSAGE-----
    Hash: SHA1

    Stefan Rybacki schreef:
    > Tom Anderson schrieb:
    >> 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.

    >
    > I don't really like this approach because it means you have to adapt the
    > interface whenever you add a new Constraint type which in return means
    > you need to adjust all implemented to that changes.


    The complexity has to be somewhere though. Right now, the problematic
    part is the instanceofs, this way, you shift it to the Handler
    interface. But since you control that interface, it is much easier to
    do that. In any case, you need *some* place to keep track of the
    complexity.

    2¢, H.
    - --
    Hendrik Maryns
    http://tcl.sfs.uni-tuebingen.de/~hendrik/
    ==================
    Ask smart questions, get good answers:
    http://www.catb.org/~esr/faqs/smart-questions.html
    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v2.0.9 (GNU/Linux)
    Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

    iEYEARECAAYFAkk4GOwACgkQBGFP0CTku6MnDQCcDzYXXTHkL5QmDeiFbrH2RNAq
    7HYAn0Rg6th1XKFZgcdCZZBsJCraWqCB
    =2K2U
    -----END PGP SIGNATURE-----
     
    Hendrik Maryns, Dec 4, 2008
    #13
  14. François Grondin

    Tom Anderson Guest

    On Thu, 4 Dec 2008, Stefan Rybacki wrote:

    > Tom Anderson schrieb:
    >> 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.

    >>
    >> 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) ;
    >> }

    >
    > I don't really like this approach because it means you have to adapt the
    > interface whenever you add a new Constraint type which in return means
    > you need to adjust all implemented to that changes.


    No, you need to adjust it when you add a new RegulatedNME type. And
    whatever your solution, you need to adjust things when you do that. It has
    no connection to the constraint types at all.

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

    >
    > I like the idea of using factories and a global registry though, because
    > that is what we use over here in some cases as well and it is powerful,
    > easy to maintain und to extend you can even drive the registry using
    > reflection or by external files (factory.xml -> pointing to a factory to
    > register) that can be scanned on startup.


    It's no more powerful than any other approach suggested, i think it's far
    harder to maintain than the ones which exploit static typing, and i don't
    see how it's any easier to make configurable than one of the static
    solutions. For example, you could easily write a handler implementation
    which read its list of constraints from a config file, rather than having
    them hardcoded.

    tom

    --
    I'm angry, but not Milk and Cheese angry. -- Mike Froggatt
     
    Tom Anderson, Dec 4, 2008
    #14
  15. "Tom Anderson" <> a écrit dans le message de news:
    ...
    > 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 agree that 'I' may not be conventional, but I work for a group of software
    developers that chose to prefix interfaces with 'I'. And 'NME' is short for
    'Network Modeling Element', which is meaningful in my area (real-time
    control of sewer networks).

    >>> 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.


    I'm aware of that. But doing that in my context creates a circular
    dependancy between two packages... That's why I didn't want to go that way.

    >>> 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




    Tom, I thank you very much for your last two solutions. This is exactly what
    I'm looking for. I'll probably go with the Visitor-like pattern.

    BTW, no problem with the cedilla. And also, thanks for your reply to Stefan
    Ram. He was right about the parentheses, but as you mentioned, the lines
    were understandable. You read my mind.

    Best regards.

    Francois
     
    François Grondin, Dec 4, 2008
    #15
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Digital Puer

    when to use instanceof?

    Digital Puer, Sep 4, 2003, in forum: Java
    Replies:
    3
    Views:
    4,136
    Roedy Green
    Sep 5, 2003
  2. VisionSet
    Replies:
    13
    Views:
    773
    Dale King
    Nov 18, 2003
  3. HalcyonWild
    Replies:
    30
    Views:
    1,294
  4. Replies:
    21
    Views:
    22,238
  5. Pavel
    Replies:
    2
    Views:
    344
    Pavel
    Apr 18, 2009
Loading...

Share This Page