|
Peppe, RasterizePlugIn is an interesting addition. Maybe you have not completely finished, sorry,
I saw this today and had some time in the train to make a quick review : - plugin is not activated yet (not in default-plugin.xml)
- not fully internationalized (method getName)
- would be nice to have more comments/javadoc
- plugin throws a NPE if selected attribute contains nulls
- RasterizeAlgorithm class : why don't you stick to CamelCase conventions for method names ? About the UI :
The layer combobox is added 2 times, one with the MultiInputDialog
framework and one is created manually and added with MultiFormUtils.
Why did you try to manage the LayerComboBox manually ?
Try to stick to MultiInputDialog as much as possible. Missing capabilities
may be discussed on the list About design or code details
You added new methods in FeatureCollectionUtil to validate and union
features, but methods are 80% redundant with existing plugins.
- Making a geometry valid is a single method call : maybe it is not
useful to iterate several times through the collection to make geometries
valid, then to union them
- Union is a costly operation : would be interesting to check that
unioning before rasterizing is worthwhile (I suppose rasterizing
each single feature would produce the same result, even in case or overlaps).
Is it faster to union before or do you get a different result ?
- You make features valid without cloning them : it means you change actual geometries.
Are you sure the user want to change its geometries ? Cannot be reverted.
- UnionByAttributeValue : you only need geometry and an attribute for the result schema.
Other attributes are undefined (null in your case). Not a big problem though.
- UnionByAttributeValue : line 141 do nothing
featureCollection.getFeatures().get(0).getGeometry().getFactory();
- UnionByAttributeValue : lines 146, 147 do nothing useful
(using newSchema after that is just like using schema)
FeatureSchema newSchema;
newSchema = schema; Michaël
|
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
