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

[Vincent Hennebert]

Hi Adrian,

Nice job! Two nits:

> Author: acumiskey
> Date: Tue May  6 09:14:09 2008
> New Revision: 653826
<snip/>
> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/apps/FopFactory.java
> URL: 
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/apps/FopFactory.java?rev=653826&r1=653825&r2=653826&view=diff
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/apps/FopFactory.java 
> (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/apps/FopFactory.java Tue 
> May  6 09:14:09 2008
> +    /**
> +     * 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.


> Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/render/PrintRenderer.java
> URL: 
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/PrintRenderer.java?rev=653826&r1=653825&r2=653826&view=diff
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/render/PrintRenderer.java 
> (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/render/PrintRenderer.java 
> Tue May  6 09:14:09 2008
>      /**
> -     * adds a font list to current list of fonts
> -     * @param fontInfoList font list
> +     * Adds a font list to current list of fonts
> +     * @param fontList a font info list
>       */
> -    public void addFontList(List fontInfoList) {
> -        if (this.fontList == null) {
> -            setFontList(fontInfoList);
> +    public void addFontList(List/*<EmbedFontInfo>*/ fontList) {
> +        if (embedFontInfoList == null) {
> +            setFontList(fontList);
>          } else {
> -            this.fontList.addAll(fontInfoList);
> +            fontList.addAll(fontList);

I have a slight doubt here. It’s not that concatenating a list to itself
doesn’t have any interest, but... ;-) Shouldn’t it be embedFontInfoList instead?


Thanks,
Vincent


-- 
Vincent Hennebert                            Anyware Technologies
http://people.apache.org/~vhennebert         http://www.anyware-tech.com
Apache FOP Committer                         FOP Development/Consulting