Re: svn commit: r1068509 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java

[Andreas Delmelle <an...@telenet.be>]

On 08 Feb 2011, at 19:32, adelmelle@apache.org wrote:

> Author: adelmelle
> Date: Tue Feb  8 18:32:15 2011
> New Revision: 1068509
> 
> URL: http://svn.apache.org/viewvc?rev=1068509&view=rev
> Log:
> Elimination of code duplication in getNextKnuthElements()

Finally managed to really factor out the duplication that was bugging me.
Took me a while to figure out exactly what was different in the restart-cases, but I believe I got it nailed down now.
The trick was to alter the main loop slightly, and move the initial assignment of currentChildLM out of the condition, and create a sort of 'initial child LM setup' block before the main while-loop (lines 275 - 287).

Before my changes (incl. those made during the weekend), basically the same code block was repeated in:
- the regular getNextKnuthElements() method (while-loop)
- the restart-variant of getNextKnuthElements()
  * the processing of children of the initial LM after a restart
  * the processing of the other childLMs after a restart (while-loop)

That (obviously) significantly increased the chances of misalignment between the three different scenarios...
The block in question, however, has multiple exit points. This makes it non-trivial to extract as a private method, so I can see how at least some duplication might have seemed unavoidable at the time of implementing.

Still, after giving it some thought, all three occurrences can basically be trimmed down to the latter with some minimal additional if-else logic, and the 'regular' case actually becomes a special variant of a restart (= without a stack of active LMs).


Regards,

Andreas
---