Re: Problem in PageBreakingAlgorithm Constructor

2009-10-26 Thread Alexander Kiel
Hi Vincent,

thanks for sorting this out.

Best Regards
Alexander

On Mon, 2009-10-26 at 12:18 +, Vincent Hennebert wrote:
> Hi Alexander,
> 
> That piece of code didn’t make sense, in addition to being wrong. It’s
> up to the user to add stretching to the footnote separator if they ever
> want to. Actually that code could even have led to unexpected results in
> some cases.
> 
> Since it wasn’t breaking any layout test case, I removed it altogether.
> 
> Thanks,
> Vincent
> 
> 
> Alexander Kiel wrote:
> > Hi,
> > 
> > the constructor of the class PageBreakingAlgorithm looks like this:
> > 
> > public PageBreakingAlgorithm(LayoutManager topLevelLM,
> >  PageProvider pageProvider,
> >  PageBreakingLayoutListener
> > layoutListener,
> >  int alignment, int alignmentLast,
> >  MinOptMax footnoteSeparatorLength,
> >  boolean partOverflowRecovery, boolean
> > autoHeight,
> >  boolean favorSinglePart) {
> > super(alignment, alignmentLast, true, partOverflowRecovery, 0);
> > this.topLevelLM = topLevelLM;
> > this.pageProvider = pageProvider;
> > this.layoutListener = layoutListener;
> > best = new BestPageRecords();
> > this.footnoteSeparatorLength = (MinOptMax)
> > footnoteSeparatorLength.clone();
> > // add some stretch, to avoid a restart for every page
> > containing footnotes
> > if (footnoteSeparatorLength.min == footnoteSeparatorLength.max)
> > {
> > footnoteSeparatorLength.max += 1;
> > }
> > this.autoHeight = autoHeight;
> > this.favorSinglePart = favorSinglePart;
> > }
> > 
> > The problem is the line:
> > 
> > footnoteSeparatorLength.max += 1;
> > 
> > I think it should read rather:
> > 
> > this.footnoteSeparatorLength.max += 1;
> > 
> > Clients calling the constructor shouldn't be happy about this situation.
> > 
> > I discovered this statement while refactoring the MinOptMax class into
> > an immutable one. I think this refactoring project should be another
> > mail. But this example shows how valuable a immutable MinOptMax would
> > be.
> > 
> > Can someone familiar with this part of FOP write a test which fails
> > against this current behavior? I could than use this test to verify that
> > my immutable MinOptMax works with this part.
> > 
> > 
> > Thanks
> > Alex
> > 
> 



signature.asc
Description: This is a digitally signed message part


Re: Problem in PageBreakingAlgorithm Constructor

2009-10-26 Thread Vincent Hennebert
Hi Alexander,

That piece of code didn’t make sense, in addition to being wrong. It’s
up to the user to add stretching to the footnote separator if they ever
want to. Actually that code could even have led to unexpected results in
some cases.

Since it wasn’t breaking any layout test case, I removed it altogether.

Thanks,
Vincent


Alexander Kiel wrote:
> Hi,
> 
> the constructor of the class PageBreakingAlgorithm looks like this:
> 
> public PageBreakingAlgorithm(LayoutManager topLevelLM,
>  PageProvider pageProvider,
>  PageBreakingLayoutListener
> layoutListener,
>  int alignment, int alignmentLast,
>  MinOptMax footnoteSeparatorLength,
>  boolean partOverflowRecovery, boolean
> autoHeight,
>  boolean favorSinglePart) {
> super(alignment, alignmentLast, true, partOverflowRecovery, 0);
> this.topLevelLM = topLevelLM;
> this.pageProvider = pageProvider;
> this.layoutListener = layoutListener;
> best = new BestPageRecords();
> this.footnoteSeparatorLength = (MinOptMax)
> footnoteSeparatorLength.clone();
> // add some stretch, to avoid a restart for every page
> containing footnotes
> if (footnoteSeparatorLength.min == footnoteSeparatorLength.max)
> {
> footnoteSeparatorLength.max += 1;
> }
> this.autoHeight = autoHeight;
> this.favorSinglePart = favorSinglePart;
> }
> 
> The problem is the line:
> 
> footnoteSeparatorLength.max += 1;
> 
> I think it should read rather:
> 
> this.footnoteSeparatorLength.max += 1;
> 
> Clients calling the constructor shouldn't be happy about this situation.
> 
> I discovered this statement while refactoring the MinOptMax class into
> an immutable one. I think this refactoring project should be another
> mail. But this example shows how valuable a immutable MinOptMax would
> be.
> 
> Can someone familiar with this part of FOP write a test which fails
> against this current behavior? I could than use this test to verify that
> my immutable MinOptMax works with this part.
> 
> 
> Thanks
> Alex
>