Hi Michael, thanks for the comment. Indeed my idea was to test and refine the plugin before to make available. *- plugin throws a NPE if selected attribute contains nulls* Thanks, I corrected it
*- RasterizeAlgorithm class : why don't you stick to CamelCase conventions for method names?* I will change the names of he methods *The layer combobox is added 2 times, one with the MultiInputDialogframework 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 capabilitiesmay be discussed on the list* I changed. MultiInputDialog is more easy to use. I like anyhow the idea to distinguish the type of target layer (vector or images or raster image) *Making a geometry valid is a single method call : maybe it is notuseful to iterate several times through the collection to make geometriesvalid, then to union them* Later I realized that valid operation is not really needed. In the next RasterizePlugin it will be excluded saving some seconds *Union is a costly operation : would be interesting to check thatunioning before rasterizing is worthwhile (I suppose rasterizingeach 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 *? Sextante Rasterize algorithm is more efficient and faster also if there are few geometries to rasterize, no matter if they are big or small. A union, even if it takes some resources from memory, will drastically reduce memory consumption and time during rasterization. I also realized that a clipping/selection operation limited to the features to rasterize at the beginning of the process will save time and resources. *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;* Removed. Thanks. Peppe Il giorno mar 22 set 2020 alle ore 19:07 Michaud Michael < m.michael.mich...@orange.fr> ha scritto: > 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 >
_______________________________________________ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
