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...)

If that is possible, the check can be removed from the method. If the member is also declared as volatile, the method itself does not need to be synchronized, since all threads will have the same view of that variable.


Just my 2 cents


Cheers

Andreas

Reply via email to