This feedback may be over kill for an internal refactoring, but I find the
number of parameters hanging off the identify method to be awkward.

You already have one "param object" in FeatureInfoRequestParameters -
excellent move as WMS specification will change over time.

Should  layer, style, filter, maxFeatures (which all seem to provide some
context) be handled the same way under to allow growth in the future
without breaking API?

--
Jody


Jody Garnett


On Wed, Nov 27, 2013 at 4:49 AM, Andrea Aime
<andrea.a...@geo-solutions.it>wrote:

> Hi,
> this pull request, https://github.com/geoserver/geoserver/pull/407/files,
> sets the stage for the new vector GetFeatureInfo code.
>
> In particular, it splits the highly procedural code in GetFeatureInfo in a
> set of
> classes, each implementing a feature info code path for a particular type
> of layer,
> and making it extensible via this new extension point:
>
> /**
>  * Extension point that helps run GetFeatureInfo on a specific layer
>  *
>  * @author Andrea Aime - GeoSolutions
>  */
> public interface LayerIdentifier {
>
>     /**
>      * Returns true if the identifier can handle this layer, false
> otherwise
>      *
>      * @param layer
>      * @return
>      */
>     boolean canHandle(MapLayerInfo layer);
>
>     /**
>      * Returns a feature collection identifying the "features" found at
> the specified location
>      *
>      * @param params The request parameters
>      * @param layer The layer being queried
>      * @param style The style for this layer
>      * @param filter An eventual additional filter
>      * @param maxFeatures Max number of features to be returned
>      * @return A list of FeatureCollection objects, each feature in them
> represent an item the user
>      *         clicked on
>      * @throws Exception
>      */
>     List<FeatureCollection> identify(FeatureInfoRequestParameters params,
> MapLayerInfo layer,
>             Style style, Filter filter, int maxFeatures) throws Exception;
>
> }
>
> The refactor seems large, but it's just shuffling code around, which was
> already almost cleanly
> separated in different private methods of GetFeatureInfo
>
> Objections? Suggestions for improvement?
>
> Cheers
> Andrea
>
> --
> ==
> Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
> information.
> ==
>
> Ing. Andrea Aime
> @geowolf
> Technical Lead
>
> GeoSolutions S.A.S.
> Via Poggio alle Viti 1187
> 55054  Massarosa (LU)
> Italy
> phone: +39 0584 962313
> fax: +39 0584 1660272
> mob: +39  339 8844549
>
> http://www.geo-solutions.it
> http://twitter.com/geosolutions_it
>
> -------------------------------------------------------
>
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics
> Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> _______________________________________________
> Geoserver-devel mailing list
> Geoserver-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
>
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to