Further cleanup/readability improvements

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@790166 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 93 insertions, 56 deletions

diff --git a/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java b/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
index 624bc907a..a8a35af9a 100644
--- a/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
+++ b/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java
@@ -562,7 +562,7 @@ public abstract class AbstractBreaker {
             nextSequenceStartsOn = handleSpanChange(childLC, nextSequenceStartsOn);
 
             Position breakPosition = null;
-            if (((KnuthElement) ListUtil.getLast(returnedList)).isForcedBreak()) {
+            if (ElementListUtils.endsWithForcedBreak(returnedList)) {
                 KnuthPenalty breakPenalty = (KnuthPenalty) ListUtil
                         .removeLast(returnedList);
                 breakPosition = breakPenalty.getPosition();
diff --git a/src/java/org/apache/fop/layoutmgr/PageBreaker.java b/src/java/org/apache/fop/layoutmgr/PageBreaker.java
index 20fd89cd9..299a537dc 100644
--- a/src/java/org/apache/fop/layoutmgr/PageBreaker.java
+++ b/src/java/org/apache/fop/layoutmgr/PageBreaker.java
@@ -350,7 +350,7 @@ public class PageBreaker extends AbstractBreaker {
                     1, true, BreakingAlgorithm.ALL_BREAKS);
         AbstractBreaker.log.debug("restart: optimalPageCount= " + optimalPageCount
                 + " pageBreaks.size()= " + algRestart.getPageBreaks().size());
-        
+
         boolean fitsOnePage
                 = optimalPageCount <= pslm.getCurrentPV().getBodyRegion().getColumnCount();
 
@@ -418,7 +418,7 @@ public class PageBreaker extends AbstractBreaker {
             || pbp.footnoteFirstElementIndex <= pbp.footnoteLastElementIndex) {
             // call addAreas() for each FootnoteBodyLM
             for (int i = pbp.footnoteFirstListIndex; i <= pbp.footnoteLastListIndex; i++) {
-                LinkedList elementList = alg.getFootnoteList(i);
+                List elementList = alg.getFootnoteList(i);
                 int firstIndex = (i == pbp.footnoteFirstListIndex
                         ? pbp.footnoteFirstElementIndex : 0);
                 int lastIndex = (i == pbp.footnoteLastListIndex
@@ -542,4 +542,4 @@ public class PageBreaker extends AbstractBreaker {
             return true;
         }
     }
-}
\ No newline at end of file
+}
diff --git a/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java b/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
index f92c31178..469784a1b 100644
--- a/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
+++ b/src/java/org/apache/fop/layoutmgr/PageBreakingAlgorithm.java
@@ -20,16 +20,19 @@
 package org.apache.fop.layoutmgr;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.apache.fop.fo.Constants;
 import org.apache.fop.fo.FObj;
 import org.apache.fop.layoutmgr.AbstractBreaker.PageBreakPosition;
 import org.apache.fop.traits.MinOptMax;
+import org.apache.fop.util.ListUtil;
 
 class PageBreakingAlgorithm extends BreakingAlgorithm {
 
@@ -47,9 +50,9 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
      * List&lt;List&lt;KnuthElement&gt;&gt;, it contains the sequences of KnuthElement
      * representing the footnotes bodies.
      */
-    private ArrayList footnotesList = null;
+    private List footnotesList = null;
     /** Cumulated bpd of unhandled footnotes. */
-    private ArrayList lengthList = null;
+    private List lengthList = null;
     /** Length of all the footnotes which will be put on the current page. */
     private int totalFootnotesLength = 0;
     /**
@@ -58,6 +61,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
      * footnotes from its preceding pages.
      */
     private int insertedFootnotesLength = 0;
+
     /** True if footnote citations have been met since the beginning of the page sequence. */
     private boolean footnotesPending = false;
     /**
@@ -178,6 +182,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
     }
 
+    /** {@inheritDoc} */
     protected void initialize() {
         super.initialize();
         insertedFootnotesLength = 0;
@@ -185,6 +190,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         footnoteElementIndex = -1;
     }
 
+    /** {@inheritDoc} */
     protected KnuthNode createNode(int position, int line, int fitness,
                                    int totalWidth, int totalStretch, int totalShrink,
                                    double adjustRatio, int availableShrink, int availableStretch,
@@ -196,6 +202,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                                  difference, totalDemerits, previous);
     }
 
+    /** {@inheritDoc} */
     protected KnuthNode createNode(int position, int line, int fitness,
                                    int totalWidth, int totalStretch, int totalShrink) {
         return new KnuthPageNode(position, line, fitness,
@@ -209,6 +216,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
     }
 
     /**
+     * {@inheritDoc}
      * Page-breaking specific handling of the given box. Currently it adds the footnotes
      * cited in the given box to the list of to-be-handled footnotes.
      * @param box a block-level element possibly containing foonotes citations
@@ -245,9 +253,9 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
 
         // compute the total length of the footnotes
-        ListIterator elementListsIterator = elementLists.listIterator();
-        while (elementListsIterator.hasNext()) {
-            LinkedList noteList = (LinkedList) elementListsIterator.next();
+        for (Iterator elementListsIterator = elementLists.iterator();
+                elementListsIterator.hasNext();) {
+            final List noteList = (List) elementListsIterator.next();
 
             //Space resolution (Note: this does not respect possible stacking constraints
             //between footnotes!)
@@ -255,21 +263,23 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
 
             int noteLength = 0;
             footnotesList.add(noteList);
-            ListIterator noteListIterator = noteList.listIterator();
-            while (noteListIterator.hasNext()) {
-                KnuthElement element = (KnuthElement) noteListIterator.next();
+            for (Iterator noteListIterator = noteList.iterator();
+                    noteListIterator.hasNext();) {
+                final KnuthElement element = (KnuthElement) noteListIterator.next();
                 if (element.isBox() || element.isGlue()) {
                     noteLength += element.getW();
                 }
             }
-            int prevLength = (lengthList.size() == 0
+            int prevLength = (lengthList == null || lengthList.isEmpty())
                     ? 0
-                    : ((Integer) lengthList.get(lengthList.size() - 1)).intValue());
+                    : ((Integer) ListUtil.getLast(lengthList)).intValue();
+            //TODO: replace with Integer.valueOf() once we switch to Java 5
             lengthList.add(new Integer(prevLength + noteLength));
             totalFootnotesLength += noteLength;
         }
     }
 
+    /** {@inheritDoc} */
     protected int restartFrom(KnuthNode restartingNode, int currentIndex) {
         int returnValue = super.restartFrom(restartingNode, currentIndex);
         newFootnotes = false;
@@ -277,10 +287,10 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
             // remove from footnotesList the note lists that will be met
             // after the restarting point
             for (int j = currentIndex; j >= restartingNode.position; j--) {
-                KnuthElement resettedElement = getElement(j);
-                if (resettedElement instanceof KnuthBlockBox
-                    && ((KnuthBlockBox) resettedElement).hasAnchors()) {
-                    resetFootnotes(((KnuthBlockBox) resettedElement).getElementLists());
+                final KnuthElement resetElement = getElement(j);
+                if (resetElement instanceof KnuthBlockBox
+                    && ((KnuthBlockBox) resetElement).hasAnchors()) {
+                    resetFootnotes(((KnuthBlockBox) resetElement).getElementLists());
                 }
             }
         }
@@ -289,12 +299,12 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
 
     private void resetFootnotes(List elementLists) {
         for (int i = 0; i < elementLists.size(); i++) {
-            /*LinkedList removedList = (LinkedList)*/footnotesList.remove(footnotesList.size() - 1);
-            lengthList.remove(lengthList.size() - 1);
+            /*LinkedList removedList = (LinkedList)*/ListUtil.removeLast(footnotesList);
+            ListUtil.removeLast(lengthList);
 
             // update totalFootnotesLength
-            if (lengthList.size() > 0) {
-                totalFootnotesLength = ((Integer) lengthList.get(lengthList.size() - 1)).intValue();
+            if (!lengthList.isEmpty()) {
+                totalFootnotesLength = ((Integer) ListUtil.getLast(lengthList)).intValue();
             } else {
                 totalFootnotesLength = 0;
             }
@@ -305,6 +315,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
     }
 
+    /** {@inheritDoc} */
     protected void considerLegalBreak(KnuthElement element, int elementIdx) {
         super.considerLegalBreak(element, elementIdx);
         newFootnotes = false;
@@ -314,7 +325,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                                     int elementIndex) {
         KnuthPageNode pageNode = (KnuthPageNode) activeNode;
         int actualWidth = totalWidth - pageNode.totalWidth;
-        int footnoteSplit;
+        int footnoteSplit = 0;
         boolean canDeferOldFootnotes;
         if (element.isPenalty()) {
             actualWidth += element.getW();
@@ -333,7 +344,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                     insertedFootnotesLength = pageNode.totalFootnotes + allFootnotes;
                     footnoteListIndex = footnotesList.size() - 1;
                     footnoteElementIndex
-                        = ((LinkedList) footnotesList.get(footnoteListIndex)).size() - 1;
+                        = getFootnoteList(footnoteListIndex).size() - 1;
                 } else if (((canDeferOldFootnotes
                                 = checkCanDeferOldFootnotes(pageNode, elementIndex))
                             || newFootnotes)
@@ -359,7 +370,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                     insertedFootnotesLength = pageNode.totalFootnotes + allFootnotes;
                     footnoteListIndex = footnotesList.size() - 1;
                     footnoteElementIndex
-                        = ((LinkedList) footnotesList.get(footnoteListIndex)).size() - 1;
+                        = getFootnoteList(footnoteListIndex).size() - 1;
                 }
             } else {
                 // all footnotes have already been placed on previous pages
@@ -376,7 +387,8 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
     }
 
-    /** Checks whether footnotes from preceding pages may be deferred to the page after
+    /**
+     * Checks whether footnotes from preceding pages may be deferred to the page after
      * the given element.
      * @param node active node for the preceding page break
      * @param contentElementIndex index of the Knuth element considered for the
@@ -449,7 +461,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         return ((newFootnotes
                  && firstNewFootnoteIndex != 0
                  && (listIndex < firstNewFootnoteIndex - 1
-                     || elementIndex < ((LinkedList) footnotesList.get(listIndex)).size() - 1))
+                     || elementIndex < getFootnoteList(listIndex).size() - 1))
                 || length < totalFootnotesLength);
     }
 
@@ -458,6 +470,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
      * @param activeNode currently considered previous page break
      * @param availableLength available space for footnotes
      * @param canDeferOldFootnotes
+     * @return ...
      */
     private int getFootnoteSplit(KnuthPageNode activeNode, int availableLength,
                 boolean canDeferOldFootnotes) {
@@ -474,6 +487,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
      * @param prevLength total length of footnotes inserted so far
      * @param availableLength available space for footnotes on this page
      * @param canDeferOldFootnotes
+     * @return ...
      */
     private int getFootnoteSplit(int prevListIndex, int prevElementIndex, int prevLength,
                                  int availableLength, boolean canDeferOldFootnotes) {
@@ -492,7 +506,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
             // already placed in a page: advance to the next element
             int listIndex = prevListIndex;
             int elementIndex = prevElementIndex;
-            if (elementIndex == ((LinkedList) footnotesList.get(listIndex)).size() - 1) {
+            if (elementIndex == getFootnoteList(listIndex).size() - 1) {
                 listIndex++;
                 elementIndex = 0;
             } else {
@@ -525,8 +539,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
             }
 
             // try adding a split of the next note
-            noteListIterator = ((LinkedList) footnotesList.get(listIndex))
-                    .listIterator(elementIndex);
+            noteListIterator = getFootnoteList(listIndex).listIterator(elementIndex);
 
             int prevSplitLength = 0;
             int prevIndex = -1;
@@ -540,7 +553,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                     prevIndex = index;
                 }
                 // get a sub-sequence from the note element list
-                boolean bPrevIsBox = false;
+                boolean boxPreceding = false;
                 while (noteListIterator.hasNext()) {
                     // as this method is called only if it is not possible to insert
                     // all footnotes, and we have already tried (and failed) to insert
@@ -550,31 +563,33 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                     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;
-                        }
+                        //}
                     }
                 }
             }
+
             // if prevSplitLength is 0, this means that the available length isn't enough
             // to insert even the smallest split of the last footnote, so we cannot end a
             // page here
             // if prevSplitLength is > 0 we can insert some footnote content in this page
             // and insert the remaining in the following one
+            //TODO: check this conditional, as the first one is always false...?
             if (!somethingAdded) {
                 // there was not enough space to add a piece of the first new footnote
                 // this is not a good break
@@ -584,12 +599,13 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                 footnoteListIndex = (prevIndex != -1) ? listIndex : listIndex - 1;
                 footnoteElementIndex = (prevIndex != -1)
                     ? prevIndex
-                    : ((LinkedList) footnotesList.get(footnoteListIndex)).size() - 1;
+                    : getFootnoteList(footnoteListIndex).size() - 1;
             }
             return prevSplitLength;
         }
     }
 
+    /** {@inheritDoc} */
     protected double computeAdjustmentRatio(KnuthNode activeNode, int difference) {
         // compute the adjustment ratio
         if (difference > 0) {
@@ -619,18 +635,23 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
     }
 
+    /** {@inheritDoc} */
     protected double computeDemerits(KnuthNode activeNode, KnuthElement element,
                                     int fitnessClass, double r) {
         double demerits = 0;
         // compute demerits
         double f = Math.abs(r);
         f = 1 + 100 * f * f * f;
-        if (element.isPenalty() && element.getP() >= 0) {
-            f += element.getP();
-            demerits = f * f;
-        } else if (element.isPenalty() && !element.isForcedBreak()) {
+        if (element.isPenalty()) {
             double penalty = element.getP();
-            demerits = f * f - penalty * penalty;
+            if (penalty >= 0) {
+                f += penalty;
+                demerits = f * f;
+            } else if (!element.isForcedBreak()) {
+                demerits = f * f - penalty * penalty;
+            } else {
+                demerits = f * f;
+            }
         } else {
             demerits = f * f;
         }
@@ -655,7 +676,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
             }
             if (footnoteListIndex < footnotesList.size()) {
                 if (footnoteElementIndex
-                        < ((LinkedList) footnotesList.get(footnoteListIndex)).size() - 1) {
+                        < getFootnoteList(footnoteListIndex).size() - 1) {
                     // add demerits for the footnote split between pages
                     demerits += splitFootnoteDemerits;
                 }
@@ -681,6 +702,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
     }
 
     private void createFootnotePages(KnuthPageNode lastNode) {
+
         insertedFootnotesLength = lastNode.totalFootnotes;
         footnoteListIndex = lastNode.footnoteListIndex;
         footnoteElementIndex = lastNode.footnoteElementIndex;
@@ -690,18 +712,17 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
 
         // create pages containing the remaining footnote bodies
         while (insertedFootnotesLength < totalFootnotesLength) {
+            final int tmpLength = ((Integer) lengthList.get(footnoteListIndex)).intValue();
             // try adding some more content
-            if (((Integer) lengthList.get(footnoteListIndex)).intValue() - insertedFootnotesLength
-                <= availableBPD) {
+            if ((tmpLength - insertedFootnotesLength) <= availableBPD) {
                 // add a whole footnote
-                availableBPD -= ((Integer) lengthList.get(footnoteListIndex)).intValue()
-                                - insertedFootnotesLength;
-                insertedFootnotesLength = ((Integer)lengthList.get(footnoteListIndex)).intValue();
+                availableBPD -= tmpLength - insertedFootnotesLength;
+                insertedFootnotesLength = tmpLength;
                 footnoteElementIndex
-                    = ((LinkedList)footnotesList.get(footnoteListIndex)).size() - 1;
+                    = getFootnoteList(footnoteListIndex).size() - 1;
             } else if ((split = getFootnoteSplit(footnoteListIndex, footnoteElementIndex,
-                                                 insertedFootnotesLength, availableBPD, true))
-                       > 0) {
+                                     insertedFootnotesLength, availableBPD, true))
+                    > 0) {
                 // add a piece of a footnote
                 availableBPD -= split;
                 insertedFootnotesLength += split;
@@ -733,12 +754,19 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
     }
 
     /**
-     * @return a list of PageBreakPosition elements
+     * @return a list of {@link PageBreakPosition} elements
+     *          corresponding to the computed page- and column-breaks
      */
     public LinkedList getPageBreaks() {
         return pageBreaks;
     }
 
+    /**
+     * Insert the given {@link PageBreakPosition} as the first
+     * element in the list of page-breaks
+     *
+     * @param pageBreak the position to insert
+     */
     public void insertPageBreakAsFirst(PageBreakPosition pageBreak) {
         if (pageBreaks == null) {
             pageBreaks = new LinkedList();
@@ -760,9 +788,11 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         }
     }
 
+    /** {@inheritDoc} */
     public void updateData1(int total, double demerits) {
     }
 
+    /** {@inheritDoc} */
     public void updateData2(KnuthNode bestActiveNode,
                             KnuthSequence sequence,
                             int total) {
@@ -808,7 +838,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
         int firstListIndex = ((KnuthPageNode) bestActiveNode.previous).footnoteListIndex;
         int firstElementIndex = ((KnuthPageNode) bestActiveNode.previous).footnoteElementIndex;
         if (footnotesList != null
-            && firstElementIndex == ((LinkedList) footnotesList.get(firstListIndex)).size() - 1) {
+            && firstElementIndex == getFootnoteList(firstListIndex).size() - 1) {
             // advance to the next list
             firstListIndex++;
             firstElementIndex = 0;
@@ -830,6 +860,7 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                 ratio, difference));
     }
 
+    /** {@inheritDoc} */
     protected int filterActiveNodes() {
         // leave only the active node with fewest total demerits
         KnuthNode bestActiveNode = null;
@@ -849,11 +880,17 @@ class PageBreakingAlgorithm extends BreakingAlgorithm {
                 }
             }
         }
-        return bestActiveNode.line;
+        return (bestActiveNode == null) ? -1 : bestActiveNode.line;
     }
 
-    public LinkedList getFootnoteList(int index) {
-        return (LinkedList) footnotesList.get(index);
+    /**
+     * Obtain the element-list corresponding to the footnote at the given index.
+     *
+     * @param index the index in the list of footnotes
+     * @return  the element-list
+     */
+    protected final List getFootnoteList(int index) {
+        return (List) footnotesList.get(index);
     }
 
     /** @return the associated top-level formatting object. */