xmlgraphics-fop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andreas Delmelle <andreas.delme...@telenet.be>
Subject Re: svn commit: r742766 - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/pdf: PDFT1Stream.java PDFTTFStream.java
Date Tue, 10 Feb 2009 19:26:19 GMT
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...

Mime
View raw message