Hi Andreas, The changes generally look good. I just have two small questions:
> Author: adelmelle > Date: Wed Jul 1 13:52:20 2009 > New Revision: 790166 > > URL: http://svn.apache.org/viewvc?rev=790166&view=rev > Log: > Further cleanup/readability improvements > > @@ -550,31 +563,33 @@ > if (element.isBox()) { > // element is a box > splitLength += element.getW(); > - bPrevIsBox = true; > + boxPreceding = true; > } else if (element.isGlue()) { > // element is a glue > - if (bPrevIsBox) { > + if (boxPreceding) { > // end of the sub-sequence > index = noteListIterator.previousIndex(); > break; > } > - bPrevIsBox = false; > + boxPreceding = false; > splitLength += element.getW(); > } else { > // element is a penalty > - if (element.getP() < KnuthElement.INFINITE) { > + //if (element.getP() < KnuthElement.INFINITE) { > // end of the sub-sequence > index = noteListIterator.previousIndex(); > break; > - } > + //} Why did you comment out the test? Unless I missed something that may result into regressions with some test cases. <snip/> > + /** {...@inheritdoc} */ > protected int filterActiveNodes() { > // leave only the active node with fewest total demerits > KnuthNode bestActiveNode = null; > @@ -849,11 +880,17 @@ > } > } > } > - return bestActiveNode.line; > + return (bestActiveNode == null) ? -1 : bestActiveNode.line; Why did you add this test? Did you run into NPE with some test cases? Thanks, Vincent
