Hi guys,
Sorry for jumping into this thread late... you guys are working too hard
for a weekend :).
Andrea Aime wrote:
> Hi all,
> I've reviewed Justin work, Jody featureland document, and this
> is my take on them. Warning, long email, thought most of what
> I'm going to say is not earth shattering.
>
> About the javadocs
> --------------------------------------------------------------
> First off, Justin, nicely done, the Property/Attribute/... javadoc is a
> pleasure to read, short clear and to the point, now and most things make
> sense right away. The PropertyType/AttributeType/... still seems
> to be wacky thought.
>
> Ok, from top to bottom, some comments on the amended interfaces.
> * PropertyType.getRestrictions(): List<Filter>.
> Does this list include the parent restrictions? Would be nice so.
> Given that we have and filters, couldn't this method be just
> PropertyType.getRestrictions(): Filter?
> Pro: easier to test if the value is a good one
> Cons: no way to tell which filter failed (to make this useful
> we would have to make sure Filter toString() returns a good
> representation of it, something we already have in our
> implementations if I'm correct.
>
I can see the argument for bringing the parent filters down... and it
should probably be consistent with how we bring other things down from
parent types. Like properties for instance.
> * AssociationType javadoc.
> We should have the notion of simple association in the javadoc
> as well... I mean, a business partner association is not
> aggregation and I may not be interested in making the spatial
> or temporal side of it explicit.
> Javadoc still uses first person ("I am treating the domain
> of the association type...").
> AssociationType.getRestrictions() is simply repeated, no
> extra info on it compared to the base class, same goes
> for isAbstract().
> If both Association and Attribute can be identified, can't
> we move that bit of metadata on PropertyType?
I actually haven't run through associations so those docs are probably
still rough.
>
> * AttributeType javadoc... ugly.
Missed this one as well. Will fix up similar to the others.
>
> * ComplexType
> In the javadoc, the bar property descriptor should report
> nillable == false.
>
fixed
> * SimpleFeatureType
> "attributes are non qualified". Hum, what do we do with
> GML Feature attributes like gml:name?
This is tough. I noticed in a later thread that Jody says that this
shoule be a mapping issue, i tend to agree. Ie, the attribute itself is
non-qualified, and just "name", and the gml encoder recognizes that
"name" is a special element and knows to encode it in the gml namespace.
> "the multiplicity of attributes is always assumed to be
> 1, ie, getMinOccurs() == 1 and getMaxOccurs() == 1."
> What, no nullable attributes anymore? We can't even
> represent a flat database table this way?
Also i think addressed in subsequent reply? The attribute is always
there just with a null value. I think the confusing aspect is that the
wfs spec says that when a value is null just not to include it, as
opposed to including it with an xml "nil".
>
> * PropertyDescriptor.
> Nullable or Nillable? I don't care, but let's use just one.
> A google search returns 220.000 nillable, 633.000 nullable.
> Nillable seems to come from xml schema, nullable is
> used in c# apparently.
I would say do what xml schema does, so nillable.
>
> * AssociationDescriptor.
> "attribtues"
Will fix on qa of Association interfaces
>
> * OperationType
> "exciting" should not have a place in javadocs... I guess
> this one still needs to be rewritten?
Yes... actually i would almost like to kill operations in that it is not
referenced from anywhere. Not kill the interface. My reasoning is that
we dont currently have a use case for them. Jody keeps mentioning some
coverage stuff... but noone really seems to be chomping at the bit for
it. So if it were up to me I would kill any reference to Operations from
the rest of the model, and add it when someone actually needs it.
>
> * Association
> Why not replace getRelated/setRelated with
> getValue(): Attribute, setValue(Attribute)?
> Would be more consistent with the rest of the API.
>
Now that getValue() has moved up to Property i see no reason for it
either. Especally since we can type narrow the return value. I am also
ok with keeping them in but explicity stating that those methods are
"convenience" for get/set value.
> * Attribute
> Since ComplexAttribute isA Attribute, saying that we use
> it to model simple data types is incorrect. You should say
> that simple data types should be held inside an Attribute
> instance, and that for complex attribute there are
> specific subclasses (note the change of direction in the
> implication, simple value -> attribute, but attribute
> does not imply simple value).
Good point. Will change the distinction int the javadoc.
>
> * Feature
> getID() is basically unchaged from Attribute, maybe we
> should remove it. But we should state that a Feature
> is always identified somewhere, right?
Yeah, the javdoc for FeatureType#isIdentified() states that it always
returns true. I do think the Feature.getID() javadoc does provide some
useful insight into how identifiers are generated... however we could
move that up to attribute... I am easy on this one.
>
> About this thread
> --------------------------------------------------------------
>
> Wasn't FeatureCollection supposed to be removed from factories?
> SimpleFeatureFactory.createSimpleFeatureCollection(...)
> is still there.
Yes, actually i meant to kill SimpleFeatureFactory but did not do it.
There is no gain over using it from normal FeatureFactory.
>
> One question about order. XML gives us an ordered representation
> of the properties. Does this mean that our implementation will
> use List internally to preserve that order when parsing data?
Definitely. I guess this is back to List vs Collection thing. I see no
reason not to just return a list so that people know they can access by
index, but just stating that there is no guarantee on which order the
attributes are in, unless you know where the feature has come from, like
xml, or a database table, etc...
>
> About the Feature.getDefaultGeometry(), I prefer this alternative
> compared to getDefaultGeometryProperty() but... there is a big but!
> In 2.4.x we deprecated getDefaultGeometry() and replaced with
> getPrimaryGeometry(). Now we go back?
> Do whatever you please, but please, don't ask library users to
> change that method name two times!! (same goes for SimpleFeatureType).
Yeah, i botched this one up big time. As Jody stated we needed this
review before 2.4-RC0 went out. My preference would be to revert the
changes on 2.4.x back to getDefaultGeometry().
Although... as an actual releease has gone out are we too late. I mean
the chances of a tone of client code already making the switch are slim
so I think we can probably get away with it... but it would be a nono in
terms of api policy.
>
> About Query, if we keep String to use xpath, how do we discriminate
> between attributes in different namespaces? I haven't checked
> xpath to see if there is a way to do so (most probably there is?).
Some of the namespace stuff in xpath is a bit confusing but i believe
you can do things like "//gml:name".
>
> About the destiny of FeatureCollection, I agree with ISO,
> let it be a data access tool, and forget about them being features.
Agreed 1000%. People that want to actually model a feature collection a
feature can just use feature with the appropriate attributes.
>
> About Jody featureland and wiki code examples
> --------------------------------------------------------------
>
> * Success table:
> provide operations so we can smoothly integrate with Coverage? Hmm...
> you mean, thinking about coverage as an n-dimensional function?
>
>
> * builder examples in the wiki.
> FeatureType does not have a CRS
> (it's a derived attribute, as stated by the javadoc), but the
> geometry attribute should instead.
> I fail to see the need to explicitly cast in 2.6. Can't we just
> deprecate geotools features and eventually remove them from the
> API instead?
>
>
> Ok, that's all folks :)
> Cheers
> Andrea
>
> !DSPAM:4007,46eba4a1129868992556831!
>
--
Justin Deoliveira
The Open Planning Project
http://topp.openplans.org
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
