[Gzz] Polymorphism, inheritance

[Tuomas Lukka]

org/fenfire/view/buoy/AbstractMainNode2D.java::

1.13         (tjl      16-May-03):     public boolean mouseClickHit(MouseEvent 
e, VobScene oldVobScene, float[] zout) {
1.13         (tjl      16-May-03):      float[] pt = new float[] { e.getX(), 
e.getY(), 0 };
1.13         (tjl      16-May-03):      float[] sq = new float[2];
1.13         (tjl      16-May-03):      
oldVobScene.coords.inverseTransformPoints3(
1.13         (tjl      16-May-03):              box2screen, pt, pt);
1.13         (tjl      16-May-03):      
oldVobScene.coords.getSqSize(box2screen, sq);
1.17         (mudyc    23-May-03):      if (dbg) p("sq:  1: "+sq[0]+", 3: 
"+sq[1]);
1.17         (mudyc    23-May-03):      if (this instanceof MainNode2D) {
1.17         (mudyc    23-May-03):          if(pt[0] >= 0 && pt[0] < sq[0] &&
1.17         (mudyc    23-May-03):             pt[1] >= 0 && pt[1] < sq[1]) {
1.17         (mudyc    23-May-03):              pt[2] = 0;
1.17         (mudyc    23-May-03):              oldVobScene.coords
1.17         (mudyc    23-May-03):                  
.transformPoints3(box2screen, pt, pt);
1.17         (mudyc    23-May-03):              if(zout != null)
1.17         (mudyc    23-May-03):                  zout[0] = pt[2];
1.17         (mudyc    23-May-03):              return true;
1.17         (mudyc    23-May-03):          }
1.17         (mudyc    23-May-03):      } else if (this instanceof 
FisheyeMainNode2D) {
1.17         (mudyc    23-May-03):          
oldVobScene.coords.transformPoints3(box2paper, pt, pt);
1.17         (mudyc    23-May-03):          view2d.getSize(plane, v2dwh);
1.17         (mudyc    23-May-03):          if(v2dwh[0] >= 0) {
1.17         (mudyc    23-May-03):              if(pt[0] >= 0 && pt[0] < 
v2dwh[0] &&
1.17         (mudyc    23-May-03):                 pt[1] >= 0 && pt[1] < 
v2dwh[1]) return true;
1.17         (mudyc    23-May-03):          }
1.17         (mudyc    23-May-03):      } else throw new Error("No known main 
node view2d");
1.13         (tjl      16-May-03):      return false;
1.1          (tjl      28-Apr-03):     }


There's something wrong with the code above. Before reading
below, please take a moment and think what it is.

Answer below.



























--------------------------
Answer:

This is a base class that now **knows** about its subclasses, which
is a bad thing in itself. Addtionally, it uses that knowledge
to implement different behaviour that could be done simply
by virtual methods.

If you see code in a base class that looks like

                if(this instanceof Subclass1) {
                        do A
                } else if(this instanceof Subclass2) {
                        do B
                }

then usually it would be better to write

                this.do_something()
        ..
        protected abstract void do_something();

(naturally with a descriptive name *and* documentation, and
possibly parameters) and

        class Subclass1 extends Baseclass {
                ...
                protected void do_something() {
                        do A
                }
        }

        class Subclass1 extends Baseclass {
                ...
                protected void do_something() {
                        do B
                }
        }

and explain in the subclasses why the thing is done there in a certain way.

If there is no *really* important reason to do it, checking the type
of "this" is a really bad sign - it is saying that something's wrong 
with the abstractions.

        Tuomas