Re: svn commit: r653826 [1/3] - in /xmlgraphics/fop/trunk: ./ src/documentation/content/xdocs/trunk/ src/java/org/apache/fop/apps/ src/java/org/apache/fop/fonts/ src/java/org/apache/fop/fonts/autodete

[Adrian Cumiskey]

Andreas Delmelle wrote:
On May 9, 2008, at 16:25, Vincent Hennebert wrote:

Nice job! Two nits:

I second this, and have an additional consideration:


+    /**
+     * Returns the font manager
+     * @return the font manager
+     */
+    public FontManager getFontManager() {
+        if (fontManager == null) {
+            this.fontManager = new FontManager(this);
+        }
+        return this.fontManager;
     }

Shouldn’t this method be made synchronized? IIC it might be called by
several threads.

Maybe a matter of style, but if see this, I usually move the assignment 
to the member initialization, i.e.

class FopFactory {
...
   private FontManager fontManager = new FontManager(this);
...

That is, unless there is a specific reason to wait until 
getFontManager() is called before initializing (?)
(Haven't checked whether the fontManager needs a fully initialized 
FopFactory to work properly...)

Not really of style.. just generally a design decision to instantiate objects only when they are 
called upon (on demand) rather than up front.

Adrian.