xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeremias Maerki <...@jeremias-maerki.ch>
Subject Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf: PDFT1Stream.java PDFTTFStream.java
Date Wed, 11 Feb 2009 07:46:01 GMT
On 10.02.2009 20:26:19 Andreas Delmelle wrote:
> On 10 Feb 2009, at 12:33, Vincent Hennebert wrote:
> >> Author: jeremias
> >> Date: Mon Feb  9 22:09:40 2009
> >> New Revision: 742766
> >> <snip />
> > And yet another copy-paste. Was that really impossible to factorize  
> > this
> > piece of code in a common super class?
> Heh... Well, I guess we could always /try/ to factorize out every  
> duplicate conditional. :-]
> Definitely not impossible, but since PDFTTFStream extends PDFStream  
> and PDFT1Stream extends AbstractPDFStream, this would probably lead to  
> other code being either unnecessarily duplicated or inherited, unless  
> it is carefully thought through.
> The key lies in that very simple one-liner that follows the 'if':
>      super.setupFilterList();
> Currently, there is only one common implementation in  
> AbstractPDFStream. If someone were to implement it in PDFStream  
> differently, that piece of code in question, while identical /code/  
> for both classes, could come to mean something different...
> True, it doesn't, so one could add something like an  
> AbstractPDFFontStream to hold this one small method setupFilterList().
> Since PDFTTFStream uses some members/methods in PDFStream, the choice  
> then becomes:
> * push those members/methods up (so some other classes, which do not  
> really need them, like PDFT1Stream, inherit them as well, possibly  
> with the consequence of necessitating overrides to correct behavioral  
> changes introduced by the abstract parent, and make the concrete  
> subclass act like its abstract grandparent --copy-paste?), or
> * copy what is needed from PDFStream, and paste it into PDFTTFStream,  
> then sever the link and have PDFTTFStream extend AbstractPDFFontStream
> * implement PDFTTFStream as a proxy to PDFStream. See attached patch.  
> Untested; I have not yet verified whether there other pieces of the  
> code that rely on PDFTTFStream extending PDFStream.
> That said, I also can't immediately say how many font-stream types  
> there are or should be, but if it's only two classes and it is  
> unlikely that others will become necessary, one could start to wonder  
> whether it's worth the effort...

There are two font-stream types at the moment: Type 1 and TTF.

I think it would be possible to derive PDFTTFStream from
AbstractPDFStream (or a common AbstractFontStream) without the use of
PDFStream inside its code. At the moment, the TTF data is buffered
twice: once as a byte[] in PDFFactory and once inside the PDFStream's
internal buffer. Doesn't make too much sense. I'll take a closer look
when I have time (and motivation). I have other priorities right now. If
that's a problem I can also revert revision 742766.

Jeremias Maerki

View raw message