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 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
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?
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 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
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
On Wed, Nov 27, 2013 at 6:41 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:
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?
Good idea, I've updated the pull request to follow.
The only thing that was left explicit is maxFeatures, since it's controlled
and updated as we move
from one layer to the next by the main GetFeatureInfo class.
On Thu, Nov 28, 2013 at 5:25 PM, Andrea Aime
<andrea.aime@anonymised.com>wrote:
On Wed, Nov 27, 2013 at 6:41 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:
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?
Good idea, I've updated the pull request to follow.
The only thing that was left explicit is maxFeatures, since it's
controlled and updated as we move
from one layer to the next by the main GetFeatureInfo class.