xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Cumiskey <adrian.cumis...@gmail.com>
Subject Re: svn commit: r1067533 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
Date Mon, 07 Feb 2011 12:49:45 GMT
I'd like to second Simon's comments, very unselfish and good work Andreas.

Adrian.

On 7 February 2011 16:26, Simon Pepping <spepping@leverkruid.eu> wrote:

> I am pleased that you undertake this kind of cleanup work in the
> layout engine. It is very useful that you try to make this piece of
> code more accessible.
>
> Simon
>
> On Sun, Feb 06, 2011 at 12:18:15AM +0100, Andreas Delmelle wrote:
> > On 05 Feb 2011, at 22:49, adelmelle@apache.org wrote:
> >
> > > Author: adelmelle
> > > Date: Sat Feb  5 21:49:58 2011
> > > New Revision: 1067533
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1067533&view=rev
> > > Log:
> > > Code cleanup
> > >
> > > Modified:
> > >
>  xmlgraphics/fop/trunk/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
> >
> > In preparation of decommissioning its StackingIter, I noticed I had some
> minor cleanups ready for this class. Checking closer, the code was still way
> too messy for my taste, so I went further --and further... Quite far,
> actually, so a bit of explanation needed in case someone goes looking for
> code that I removed.
> >
> > I noticed there was quite a lot of code that, in fact, was never, ever
> used. It seemed like an experiment from Luca, that likely should have been
> kept in a branch instead of being committed to the trunk in an incomplete
> state. All it did here was confuse people, in a class which is quite heavily
> used.
> > I first spent quite some time cleaning up commented log.debug()
> statements in the method createUnitElements(), then decided to check where
> the method was called, and found it was only used in one place, in
> getChangedKnuthElements():
> >
> > > -/* estensione: conversione complessiva */
> > > -/*LF*/  if (bpUnit > 0) {
> > > -/*LF*/      storedList = returnedList;
> > > -/*LF*/      returnedList = createUnitElements(returnedList);
> > > -/*LF*/  }
> >
> > Now, that bpUnit member is written *only* in BlockLayoutManager and
> BlockContainerLayoutManager, where it is set to 0 in the initialize()
> method. In effect, the method was unused, so I decided to bite the bullet
> and remove it.
> >
> > Additionally, given the above, I have also removed similar references to
> this bpUnit member in other places, which eliminates some conditional
> branches. I have not yet removed the variable itself, since it is still read
> in a few subclasses.
> >
> >
> > Regards,
> >
> > Andreas
> > ---
> >
>

Mime
View raw message