On 12.02.2009 13:35:57 Vincent Hennebert wrote:
> And that goes on...
>
> > Author: jeremias
> > Date: Wed Feb 11 14:54:26 2009
> > New Revision: 743351
> >
> > URL: http://svn.apache.org/viewvc?rev=743351&view=rev
> > Log:
> > Added missing default font setup.
> >
> > + /** {...@inheritdoc} */
> > + public void setDefaultFontInfo(FontInfo fontInfo) {
> > + FontInfo fi = Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo,
> > getUserAgent());
> > + setFontInfo(fi);
> > + }
>
> The setDefaultFontInfo method is defined 5 times in the
> AbstractBinaryWritingIFDocumentHandler hierarchy:
>
> - one in ABWIFDocumentHandler itself, here’s the code:
> FontManager fontManager =
> getUserAgent().getFactory().getFontManager();
> FontCollection[] fontCollections = new FontCollection[] {
> new Base14FontCollection(fontManager.isBase14KerningEnabled())
> };
>
> FontInfo fi = (fontInfo != null ? fontInfo : new FontInfo());
> fi.setEventListener(new
> FontEventAdapter(getUserAgent().getEventBroadcaster()));
> fontManager.setup(fi, fontCollections);
> setFontInfo(fi);
>
> - one in the AFPDocumentHandler sub-class:
> FontManager fontManager =
> getUserAgent().getFactory().getFontManager();
> FontCollection[] fontCollections = new FontCollection[] {
> new AFPFontCollection(getUserAgent().getEventBroadcaster(), null)
> };
>
> FontInfo fi = (fontInfo != null ? fontInfo : new FontInfo());
> fi.setEventListener(new
> FontEventAdapter(getUserAgent().getEventBroadcaster()));
> fontManager.setup(fi, fontCollections);
> setFontInfo(fi);
>
> - one in PCLDocumentHandler:
> FontInfo fi = Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo,
> getUserAgent());
> setFontInfo(fi);
>
> - one in TIFFDocumentHandler:
> FontInfo fi = Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo,
> getUserAgent());
> setFontInfo(fi);
>
>
> One could think: it’s not too bad, the code is duplicated only twice, in
> the last two cases it’s a one-line call to another method. Yes, but
> guess what is the code of that method:
> Graphics2D graphics2D =
> Java2DFontMetrics.createFontMetricsGraphics2D();
>
> FontManager fontManager = userAgent.getFactory().getFontManager();
> FontCollection[] fontCollections = new FontCollection[] {
> new
> org.apache.fop.render.java2d.Base14FontCollection(graphics2D),
> new InstalledFontCollection(graphics2D)
> };
>
> FontInfo fi = (fontInfo != null ? fontInfo : new FontInfo());
> fi.setEventListener(new
> FontEventAdapter(userAgent.getEventBroadcaster()));
> fontManager.setup(fi, fontCollections);
> return fi;
I'm so completely lost. I have absolutely no idea what you want to say
here. I don't see any optimization potential here at all. But I'm sure
you could do it better.
> Looking further into the ABWIFDocumentHandler hierarchy, we can see how
> the startDocument method is implemented:
> - in AFPDocumentHandler:
> try {
> if (getUserAgent() == null) {
> throw new IllegalStateException(
> "User agent must be set before starting PostScript
> generation");
> }
> if (this.outputStream == null) {
> throw new IllegalStateException("OutputStream hasn't been set
> through setResult()");
> }
> [...]
> } catch (IOException e) {
> throw new IFException("I/O error in startDocument()", e);
> }
>
> - in PCLDocumentHandler:
> try {
> if (getUserAgent() == null) {
> throw new IllegalStateException(
> "User agent must be set before starting PDF
> generation");
> }
> if (this.outputStream == null) {
> throw new IllegalStateException("OutputStream hasn't been set
> through setResult()");
> }
> [...]
> } catch (IOException e) {
> throw new IFException("I/O error in startDocument()", e);
> }
>
> - in PDFDocumentHandler:
> try {
> if (getUserAgent() == null) {
> throw new IllegalStateException(
> "User agent must be set before starting PDF
> generation");
> }
> if (this.outputStream == null) {
> throw new IllegalStateException("OutputStream hasn't been set
> through setResult()");
> }
> [...]
> } catch (IOException e) {
> throw new IFException("I/O error in startDocument()", e);
> }
>
> - in PSDocumentHandler:
> try {
> if (getUserAgent() == null) {
> throw new IllegalStateException(
> "User agent must be set before starting PostScript
> generation");
> }
> if (this.outputStream == null) {
> throw new IllegalStateException("OutputStream hasn't been set
> through setResult()");
> }
> [...]
> } catch (IOException e) {
> throw new IFException("I/O error in startDocument()", e);
> }
>
> - in TIFFDocumentHandler:
> try {
> if (getUserAgent() == null) {
> throw new IllegalStateException(
> "User agent must be set before starting PDF
> generation");
> }
> if (this.outputStream == null) {
> throw new IllegalStateException("OutputStream hasn't been set
> through setResult()");
> }
> [...]
> } catch (IOException e) {
> throw new IFException("I/O error in startDocument()", e);
> }
>
> Note the copy/paste errors, which means that only 2 out of the 5 error
> messages are correct.
> There’s no TODO statement anywhere, so I assume that this code is
> considered to be finalized. IIC this is new code: no legacy here, no
> inherited burden.
Fixed. Thanks for pointing out the oversight.
> Now I have just one question: why? What benefit does this development
> method bring that I’ve missed, and that overcomes all of the well-known
> drawbacks?
Two questions actually. But I don't know how/what to repond to that.
Jeremias Maerki
