[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



Reply via email to