On 08 Feb 2011, at 19:32, adelme...@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).



Reply via email to