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
