Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Vincent Hennebert]

Hi Jeremias,

Jeremias Maerki wrote:
 Hi Vincent
 
 On 16.02.2009 18:29:41 Vincent Hennebert wrote:
 Hi Jeremias,

 Jeremias Maerki wrote:
 On 12.02.2009 13:35:57 Vincent Hennebert wrote:
 And that goes on...
snip/
 The block of code inside setDefaultFontInfo is defined three times (at
 least). That’s two times two much. Please have a look at the attached
 patch: is there anything wrong with it?

 Of course it’s still not ideal: the font-specific methods should be
 extracted into some utility class, inside some font package. But I’ve
 focused on the DocumentHandler hierarchy only, and tried to fix what was
 wrong in it. I don’t have the time to look at the bigger picture
 unfortunately.
 
 Thanks, now I understand where you want to go.

I think it’s a good illustration of what I was saying about design flaws
BTW. The method to define was probably not a setDefaultFontInfo, but
rather a getDefaultFontCollections. A small change in the point of view
that may avoid troubles in the future.

(Abstraction made of what’s said below. If no method is needed at all in
the first place...)


 I’m not familiar with the font code, but I’m wondering about the
 necessity of this setDefaultFontInfo method in the first place. Why
 would DocumentHandlers need to be asked, by external objects, to setup
 their defaut fonts? Can’t they do that by themselves, at initialization
 or something?
 
 Good point. I guess they could do that. This evolved over time and I
 focused on different things so I didn't notice that it could be done in
 a better way.
 
snip/
 Fixed. Thanks for pointing out the oversight.
 That’s much better, but I think the IOException catch block could also
 be factorized. See attached patch (I didn’t fix indentation so that
 changes appear more clearly). Does it make the code any less readable?
 
 That's startDocument() - doStartDocument(). That would have to be done
 for all methods in the IFDocumentHandler and IFPainter in all
 implementations that produce an output stream. I'm a bit sceptical as to
 what that does to code readability.

Well if the method() - doMethod() practice is constantly used then it
becomes an easily recognizable pattern IMO. Of course, if it has to be
used absolutely everywhere... Grey area.


 But I'm not totally against it.

And I won’t push too much for it. The change you made was the most
important one.


 It just takes some time to do and right at this moment, it might not be
 the right moment to do that as it could have an impact on Jost's patch.
 After that has been processed, it will be less problematic to do. 
 
 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.
 Well, that’s problematic then. That means that you will continue to do
 that, and I will continue to complain, and that will lead us nowhere. Am
 I too picky? I’m happy to be proven wrong, if I’m given reasons. But
 frankly, why should we be happy with duplicated code in the codebase?
 Why does the fact that we refuse copy/paste programming make it “high
 coding standards”? This copy/paste error is a perfect illustration IMO.
 (Note that I don’t blame you for the copy/paste error itself, I’m the
 first one to make such errors.) I think copy/paste should be virtually
 banned in the context of code development. I quite like the “once and
 only once” argument of extreme programming actually [1].
 
 Ok, that's more concrete to answer to. Are you too picky? For my taste,
 yes. You're a purist. You want clean, beautiful code. Basically nothing
 wrong with that by itself. I on the other side am a pragmatist.

And for my taste, you’re too much of a pragmatist :-P


 I need
 to get things done. Function points over beauty. I fix things when they
 occur. Probably nothing wrong with that by itself, too.
 
 My clients' primary concern is, for example, that they get a very fast
 intermediate format. Of course, there's also a certain interest to keep
 things maintainable. I believe the new output implementations are a big
 step forward to make it easier to add new output formats or to extend
 existing ones (compared to the Renderer implementations).
 
 My clients pay me money for implementing things. If it takes too long to
 implement something (i.e. costs too much), the client won't accept the
 offer (and might just buy a commercial license of another FO processor)
 or I have to invest more of my unpaid time to compensate which means I
 earn even less than I already do in some contracts. If you have an
 employer that lets you work on FOP for just how long it takes until
 you're happy with the code (or if you have enough energy left after work),
 that's good for you. I'm not in that position. I always have to make
 compromises. Maybe the reality sits in between our two positions. I'm
 trying to concede to your view as far as is possible. But if this
 project 

Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Jeremias Maerki]

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=743351view=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
 

Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Vincent Hennebert]

Hi Jeremias,

Jeremias Maerki wrote:
 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=743351view=rev
 Log:
 Added missing default font setup.
  
 +/** {...@inheritdoc} */
 +public void setDefaultFontInfo(FontInfo fontInfo) {
 +FontInfo fi = Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo, 
 getUserAgent());
 +setFontInfo(fi);
 +}
snip/
 
 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.

The block of code inside setDefaultFontInfo is defined three times (at
least). That’s two times two much. Please have a look at the attached
patch: is there anything wrong with it?

Of course it’s still not ideal: the font-specific methods should be
extracted into some utility class, inside some font package. But I’ve
focused on the DocumentHandler hierarchy only, and tried to fix what was
wrong in it. I don’t have the time to look at the bigger picture
unfortunately.

I’m not familiar with the font code, but I’m wondering about the
necessity of this setDefaultFontInfo method in the first place. Why
would DocumentHandlers need to be asked, by external objects, to setup
their defaut fonts? Can’t they do that by themselves, at initialization
or something?


 Looking further into the ABWIFDocumentHandler hierarchy, we can see how
 the startDocument method is implemented:
snip/
 - 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.

That’s much better, but I think the IOException catch block could also
be factorized. See attached patch (I didn’t fix indentation so that
changes appear more clearly). Does it make the code any less readable?


 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.

Well, that’s problematic then. That means that you will continue to do
that, and I will continue to complain, and that will lead us nowhere. Am
I too picky? I’m happy to be proven wrong, if I’m given reasons. But
frankly, why should we be happy with duplicated code in the codebase?
Why does the fact that we refuse copy/paste programming make it “high
coding standards”? This copy/paste error is a perfect illustration IMO.
(Note that I don’t blame you for the copy/paste error itself, I’m the
first one to make such errors.) I think copy/paste should be virtually
banned in the context of code development. I quite like the “once and
only once” argument of extreme programming actually [1].

And yes, even a 5-line block of duplicated code should be considered as
highly suspicious IMO. That often reveals a higher-level design flaw.
And in the present case, I think it can be easily avoided without making
the code any more complicated. And without spending much more time on it.

We have to agree upon something. Otherwise, peer review won’t bring its
usual benefits.

Vincent


[1] http://my.safaribooksonline.com/0201844575/ch19lev1sec5

Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Vincent Hennebert]

And the attached patch _
Index: src/java/org/apache/fop/render/intermediate/AbstractBinaryWritingIFDocumentHandler.java
===
--- src/java/org/apache/fop/render/intermediate/AbstractBinaryWritingIFDocumentHandler.java	(revision 744954)
+++ src/java/org/apache/fop/render/intermediate/AbstractBinaryWritingIFDocumentHandler.java	(working copy)
@@ -19,6 +19,7 @@
 
 package org.apache.fop.render.intermediate;
 
+import java.awt.Graphics2D;
 import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -35,6 +36,8 @@
 import org.apache.fop.fonts.FontInfo;
 import org.apache.fop.fonts.FontManager;
 import org.apache.fop.fonts.base14.Base14FontCollection;
+import org.apache.fop.render.java2d.InstalledFontCollection;
+import org.apache.fop.render.java2d.Java2DFontMetrics;
 
 /**
  * Abstract base class for binary-writing {...@code IFDocumentHandler} implementations.
@@ -94,26 +97,54 @@
 }
 
 /** {...@inheritdoc} */
-public void setDefaultFontInfo(FontInfo fontInfo) {
+public final void setDefaultFontInfo(FontInfo fontInfo) {
 FontManager fontManager = getUserAgent().getFactory().getFontManager();
+FontInfo fi = (fontInfo != null ? fontInfo : new FontInfo());
+fi.setEventListener(new FontEventAdapter(getUserAgent().getEventBroadcaster()));
+fontManager.setup(fi, getDefaultFontCollections(fontManager));
+setFontInfo(fi);
+}
+
+protected abstract FontCollection[] getDefaultFontCollections(FontManager fontManager);
+
+protected final FontCollection[] getBase14FontCollections(FontManager fontManager) {
 FontCollection[] fontCollections = new FontCollection[] {
 new Base14FontCollection(fontManager.isBase14KerningEnabled())
 };
+return fontCollections;
+}
 
-FontInfo fi = (fontInfo != null ? fontInfo : new FontInfo());
-fi.setEventListener(new FontEventAdapter(getUserAgent().getEventBroadcaster()));
-fontManager.setup(fi, fontCollections);
-setFontInfo(fi);
+protected final FontCollection[] getJava2DFontCollections(FontManager fontManager) {
+Graphics2D graphics2D = Java2DFontMetrics.createFontMetricsGraphics2D();
+FontCollection[] fontCollections = new FontCollection[] {
+new org.apache.fop.render.java2d.Base14FontCollection(graphics2D),
+new InstalledFontCollection(graphics2D)
+};
+return fontCollections;
 }
 
 /** {...@inheritdoc} */
-public void startDocument() throws IFException {
+public final void startDocument() throws IFException {
 super.startDocument();
 if (this.outputStream == null) {
 throw new IllegalStateException(OutputStream hasn't been set through setResult());
 }
+try {
+doStartDocument();
+} catch (IOException e) {
+throw new IFException(I/O error in startDocument(), e);
+}
 }
 
+/**
+ * Actually implements the actions that are specific to the output format.
+ * {...@link #startDocument()} contains the common stuff and calls this method.
+ *
+ * @throws IOException in case an I/O error occurs when writing to the
+ * output stream
+ */
+abstract protected void doStartDocument() throws IOException;
+
 /** {...@inheritdoc} */
 public void endDocument() throws IFException {
 if (this.ownOutputStream) {
Index: src/java/org/apache/fop/render/bitmap/TIFFDocumentHandler.java
===
--- src/java/org/apache/fop/render/bitmap/TIFFDocumentHandler.java	(revision 744954)
+++ src/java/org/apache/fop/render/bitmap/TIFFDocumentHandler.java	(working copy)
@@ -34,14 +34,14 @@
 
 import org.apache.fop.apps.FopFactoryConfigurator;
 import org.apache.fop.apps.MimeConstants;
-import org.apache.fop.fonts.FontInfo;
+import org.apache.fop.fonts.FontCollection;
+import org.apache.fop.fonts.FontManager;
 import org.apache.fop.render.intermediate.AbstractBinaryWritingIFDocumentHandler;
 import org.apache.fop.render.intermediate.IFContext;
 import org.apache.fop.render.intermediate.IFDocumentHandlerConfigurator;
 import org.apache.fop.render.intermediate.IFException;
 import org.apache.fop.render.intermediate.IFPainter;
 import org.apache.fop.render.java2d.Java2DPainter;
-import org.apache.fop.render.java2d.Java2DUtil;
 
 /**
  * {...@code IFDocumentHandler} implementation that produces PCL 5.
@@ -102,17 +102,14 @@
 }
 
 /** {...@inheritdoc} */
-public void setDefaultFontInfo(FontInfo fontInfo) {
-FontInfo fi = Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo, getUserAgent());
-setFontInfo(fi);
+protected FontCollection[] getDefaultFontCollections(FontManager fontManager) {
+return getJava2DFontCollections(fontManager);
 }
 
 

Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Jeremias Maerki]

Hi Vincent

On 16.02.2009 18:29:41 Vincent Hennebert wrote:
 Hi Jeremias,
 
 Jeremias Maerki wrote:
  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=743351view=rev
  Log:
  Added missing default font setup.
   
  +/** {...@inheritdoc} */
  +public void setDefaultFontInfo(FontInfo fontInfo) {
  +FontInfo fi = 
  Java2DUtil.buildDefaultJava2DBasedFontInfo(fontInfo, getUserAgent());
  +setFontInfo(fi);
  +}
 snip/
  
  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.
 
 The block of code inside setDefaultFontInfo is defined three times (at
 least). That’s two times two much. Please have a look at the attached
 patch: is there anything wrong with it?
 
 Of course it’s still not ideal: the font-specific methods should be
 extracted into some utility class, inside some font package. But I’ve
 focused on the DocumentHandler hierarchy only, and tried to fix what was
 wrong in it. I don’t have the time to look at the bigger picture
 unfortunately.

Thanks, now I understand where you want to go.

 I’m not familiar with the font code, but I’m wondering about the
 necessity of this setDefaultFontInfo method in the first place. Why
 would DocumentHandlers need to be asked, by external objects, to setup
 their defaut fonts? Can’t they do that by themselves, at initialization
 or something?

Good point. I guess they could do that. This evolved over time and I
focused on different things so I didn't notice that it could be done in
a better way.

 
  Looking further into the ABWIFDocumentHandler hierarchy, we can see how
  the startDocument method is implemented:
 snip/
  - 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.
 
 That’s much better, but I think the IOException catch block could also
 be factorized. See attached patch (I didn’t fix indentation so that
 changes appear more clearly). Does it make the code any less readable?

That's startDocument() - doStartDocument(). That would have to be done
for all methods in the IFDocumentHandler and IFPainter in all
implementations that produce an output stream. I'm a bit sceptical as to
what that does to code readability. But I'm not totally against it.
It just takes some time to do and right at this moment, it might not be
the right moment to do that as it could have an impact on Jost's patch.
After that has been processed, it will be less problematic to do. 

 
  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.
 
 Well, that’s problematic then. That means that you will continue to do
 that, and I will continue to complain, and that will lead us nowhere. Am
 I too picky? I’m happy to be proven wrong, if I’m given reasons. But
 frankly, why should we be happy with duplicated code in the codebase?
 Why does the fact that we refuse copy/paste programming make it “high
 coding standards”? This copy/paste error is a perfect illustration IMO.
 (Note that I don’t blame you for the copy/paste error itself, I’m the
 first one to make such errors.) I think copy/paste should be virtually
 banned in the context of code development. I quite like the “once and
 only once” argument of extreme programming actually [1].

Ok, that's more concrete to answer to. Are you too picky? For my taste,
yes. You're a purist. You want clean, beautiful code. Basically nothing
wrong with that by itself. I on the other side am a pragmatist. I need
to get things done. Function points over beauty. I fix things when they
occur. Probably nothing wrong with that by itself, too.

My clients' primary concern is, for example, that they get a very fast
intermediate format. Of course, there's also a certain interest to keep
things maintainable. I believe the new output implementations are a big
step forward to make it easier to add new output formats or to extend

Re: svn commit: r743351 - /xmlgraphics/fop/branches/Temp_AreaTreeNewDesign/src/java/org/apache/fop/render/pcl/PCLDocumentHandler.java

[Vincent Hennebert]

And that goes on...

 Author: jeremias
 Date: Wed Feb 11 14:54:26 2009
 New Revision: 743351
 
 URL: http://svn.apache.org/viewvc?rev=743351view=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;


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