Frank I expect this is a case of not being aggressive enough with updating interfaces in GeoTools.
When FilterFactory2 was added we were very cautious about updating interfaces because of GeoAPI approval/review/feedback. Since that time we have added additional methods (such as the temporal expressions) directly to the FilterFactory interface. I think we could add the FilterFactory2 <https://docs.geotools.org/latest/javadocs/org/opengis/filter/FilterFactory2.html> methods to the FilterFactory <https://docs.geotools.org/latest/javadocs/org/opengis/filter/FilterFactory.html> interface and simplify the library. -- Jody Garnett On Sun, 16 May 2021 at 23:46, Frank Gasdorf via GeoTools-Devel < geotools-devel@lists.sourceforge.net> wrote: > 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 >
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
