Hi Adrian,

> Author: acumiskey
> Date: Tue Mar 24 15:39:15 2009
> New Revision: 757852
> 
> URL: http://svn.apache.org/viewvc?rev=757852&view=rev
> Log:
> Added some nice bean methods for pageSequenceMaster and simplePageMaster, 
> this also makes startOfNode() easier to read too.
> 
> Modified:
>     
> xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/pagination/PageSequence.java
<snip/>

I’m not sure I like this change. I tend to think that a class should not
expose public methods unless they are of immediate necessity. That helps
to reduce complexity and confusion for newcomers. If they prove to be
necessary they can always be added later on.

Also, in this case I don’t think any external object needs to know
whether the page masters for this page sequence were declared using
a single simple-page-master or a page-sequence-master. That knowledge
should be kept encapsulated inside the PageSequence class.

Finally, in the startOfNode method the get*Master method will be called
twice: once by has*Master to see if the master exists, and once to
actually get it. This is slightly overkill.

I think those new methods should at least be made private. Although the
original code simply looks fine to me...

WDYT?
Thanks,
Vincent

Reply via email to