Message received and understood, code improved. I'll pay more attention
next time. Looks like peer review really works around here. Thanks, Glen.
On 13.05.2005 01:38:24 Glen Mazza wrote:
> [EMAIL PROTECTED] wrote:
>
> >jeremias 2005/05/12 07:13:45
> >
> > Modified: src/java/org/apache/fop/layoutmgr/table Tag:
> > Temp_KnuthStylePageBreaking TableStepper.java
> > TableContentLayoutManager.java EffRow.java
> > Log:
> > Fix for ArrayIndexOutOfBoundsException when empty grid units are involved.
> >
> >
>
> Jeremias, I don't see this as a fix--you seem to be converting a
> RunTimeException into a logical error (system runs but you get bad
> output.) The latter is many more times harder and more stressful to fix
> because with an LE we have no idea where the problem is--FOTree, Layout,
> Renderers, PDF Library, user version of JDK/Adobe Acrobat, etc., etc.
> Converting RTE's into LE's IMO does not really create rigorous, robust,
> low-maintenance coding.
>
> >
> > + public GridUnit getGridUnit(int index) {
> > + if (index >= 0 && index < gridUnits.size()) {
> > + return (GridUnit)gridUnits.get(index);
> > + } else {
> > + return null;
> > + }
> > + }
> >
> >
>
> If the caller is so incompetent to be requesting grid unit #42 for a
> system with only 10 grid units--shouldn't we have FOP to halt with the
> Array Index RTE so we can get that bug quickly identified and fixed?
> (Or, if we can't fix it immediately, put a Band-Aid fix in the caller
> instead of the callee?) The quiet returning of "null" thwarts that, and
> when users start complaining about bad output due to the LE, we won't
> know where the problem is to fix it. In addition to wearing out
> committers wading through the renderers and the PDF library when an RTE
> would have told them to quickly look at the FO package, we'll also have
> to ask the users a bunch of irrelevant questions such as their versions
> of Adobe Acrobat, etc. I don't see how an LE helps us here.
>
> I mentioned this to you earlier because of a odd change you made (line
> 98 of [1]) to the Span class to create an LE instead of an RTE should
> PSLM ask for an invalid column. I don't understand your rationale--if
> PSLM is asking for the wrong columns, the output will be messed up
> anyway. Best then to choose the solution--i.e., the RTE--that allows us
> to quickly zero in on the problem.
>
> I converted your change back[2, line 94/85] to explicitly return an
> IllegalStateException, and it was good I did so--I later had an error in
> the PSLM coding, asked for an invalid column, and quickly was informed
> by the RTE what the problem was so I could immediately fix it.
>
> Thanks,
> Glen
>
> [1]
> http://cvs.apache.org/viewcvs.cgi/xml-fop/src/java/org/apache/fop/area/Span.java?r1=1.6&r2=1.6.2.1&diff_format=h
>
> [2]
> http://cvs.apache.org/viewcvs.cgi/xml-fop/src/java/org/apache/fop/area/Span.java?r1=1.6.2.1&r2=1.6.2.2&diff_format=h
Jeremias Maerki
