Hi folks,

I stumbled about the SLDParser while I wass looking for final field
initializations with CommonFactoryFinder.getXXX

However, I'm not sure about whether it was by intension but internally ther
is a cast from FilterFactory to FilterFactory2 on initailization an
ExpressionDOMParser. This looks strange because other implementations (and
contributions via SPI) might implement FilterFactory but not FilterFactory2.

In addition a filed expressionDomParser is initialized with static field FF
but not used in every method the same way. Sometimes an
ExpressionDOMParseris created while needed with the mentioned cast of
internal FilterFactory, sometimes it uses
CommonFactoryFinder.getFilterFactory2() and some methods using the internal
field  expressionDomParser

What do you think about changing the constructor as follows

    public SLDParser(StyleFactory factory, FilterFactory filterFactory) {
        this.factory = factory;
        this.ff = filterFactory;
        if (filterFactory instanceof FilterFactory2) {
            this.filterFactory2 = (FilterFactory2)filterFactory;
        } else {
            this.filterFactory2 = CommonFactoryFinder.getFilterFactory2();
        }
        this.expressionDOMParser = new ExpressionDOMParser(filterFactory2);
        this.onlineResourceLocator = new DefaultResourceLocator();
    }

and change the member expressionDOMParser to a final field. All other
places where ExpressionDOMParsers ar created should be changed to use final
field expressionDOMParser instead.

Any opinions? Thanks in advance for your feedback

--
Frank
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to