[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

GraphBuilder.java



patrick,

this is very nice code.  some comments/nits:

why don't Spaces have xmin(), etc. methods?  that would be
much clearer than:

          Space s = (Space) this.spaces.get(i);
        double[] d = s.getBoundingBox();
        double width = d[2]-d[0];
        double height = d[3]-d[1];

e.g.,

  double width = s.xmax() - s.xmin();


is space(0) documented?  what is it?

does (int) casting round correctly?  i think it
might just truncate.  so instead of

  int numSlots = (int) (percentage * this.grid.length);

you may want

  int numSlots = (int) (percentage * this.grid.length + 0.5);

which will be more accurate, especially when percentage
is small.

same thing for numCrickets, other casts to int, etc. 
perhaps you should write (or use from the math lib)
a round() function that does the right thing, even 
for negative numbers?

you don't verify that the last few entries of grid[]
are filled with non-null spaces.  what happens if
Math.random() returns a number very close to 1; will
your code throw an exception due to the undefined bbox?

similar comments about bbox[2], this is opaque code!
why not just write x() and y() methods?

the "for now, check if crickets in range" comment
is outdated.

the xcord,ycord code block is fairly opaque-- have you
validated it?

why is the c1 != c2 clause not checked first before
doing any math?

getSpaceArea() will be much more accurate if you subtract 
the centroid of the three points from each point before
doing the area calc.  (consider the usual case, when all
three points are far from the origin.)

the triangleContains() method excludes test points on
the triangle boundary.  this may be what you want, but
it should be documented.

there's something wrong with visible() or something
that it calls -- i've observed bugs (bad edges) in the
.iv output.

spaceOccludesSegment() must be better commented.

have you validated your segment intersection routine?


prof. t.