Andrea Aime ha scritto: > Citing from my review: > > "The whole model makes it explicit that when you're asking for a value, > you don't get one, but a Property instead. The first reaction is, why > would I need to get metadata along with data, isn't the feature model > enough? The javadoc does not tell, but ComplexFeature.getValue() is > returning a collection and not a list, so no ordering is implied. Well, > I did not notice but this goes all over the API. Why in the world aren't > List<xxx> used instead? That would allow for an ordering assumption > and would make things that much easier? (no need to carry both data and > metadata togheter, just return the value like SimpleFeature does?). I > guess I'm missing something, but you'll have to tell me what that is." > > Rewording this, using List instead of collection in FeatureType > would allow Feature to return the attribute values right away instead of > wrapping them into Property objects. This feels more natural to me, > simpler, and has less overhead (don't discount this last bit, we > are going to play with million of features). > Using List in both places allow positional correspondence, so if I > need to know what Feature.attributes[3] is, I just have to call > Feature.getType().getAttributes[3] and I have the descriptor without > the need to keep it straight in the data. > Use of generic "Collection" forces a mixture of data and metadata which > seems to be unnecessary evil to me. > > Oh, about the usage of Set... I don't really care. Yeah, I know the java > API has Collections.singleton(o), but there is also > Collections.singletonList(x) and Collections.singletonMap(k, v). > A collection with a single value is just that, by using Set you're > not telling anything about its singleton-ness, it's in the javadoc not > in the interface. If you really wanted to tell, you could create a class > Singleton extends Collections sporting and a single getValue(): Object > method to grab its contents. > Anyways, this is a very minor issue, the real one is the data/metadata > mix I see above.
Guys, we had some conversation on the #geoserver irc chat and I finally managed to understand why there is a need to mix data and metadata. Yet, I would appreciate people reading the conversation and providing feedback. If you think this chat is too long of confusign I can try to summarize it, just ask. ----------------------------------------------------------------------- (18.32.12) aaime: Hum, it seem the feature model review triggered a good chain reaction (18.32.17) aaime: but Jody is missing from the mix, sigh (18.32.34) aaime: I guess he's the only one willing to face the data/metadata mix issue... (18.32.59) aaime: btw, you're here. jgarnett, when we'll see your opinion on the matter? :) (18.35.35) jdeolive: aaime, can you sum up the metadata/data mix issue again? (18.35.50) jdeolive: keeping that thread straight is getting hard :) (18.35.53) aaime: Well, the question is simple (18.36.23) aaime: why Feature.getAttributes() should return Attribute (a mix of data and metadata) when it could return just Object instead (that is, only the data) (18.37.00) aaime: my only answer so far is that we need metadata because Collection does not give us any hint about the order of the returned attribtues (18.37.17) aaime: but if it was returning List, and the feature type would return List as well (18.37.25) aaime: we could assume a positional correspondence between the two (18.37.43) aaime: so metadata could stay only in FeatureType without having to pullute the data (18.38.37) jdeolive: hmm... well i do see one thing (18.38.54) jdeolive: so... if we did remove Attribute, and just have it return the Object (18.39.03) aaime: yes (18.39.05) jdeolive: what would we have to do to get at that metadata (18.39.15) jdeolive: i guess ask the feature type / complex type for it (18.39.20) aaime: feature.getType().getAttribute(x) (18.39.23) aaime: yes (18.39.54) aaime: it's more in line with what we're doing now (18.40.09) aaime: and saves at least 16bytes for evey single attribute around (18.40.10) jdeolive: right... but that kind of does not work for complex features (18.40.20) aaime: (4 bytes header for Attribute, 4 bytes reference to descriptor) (18.40.26) aaime: (sorry, then 8 bytes) (18.40.29) jdeolive: because attribute to type is not one to one anymore is it not? (18.40.59) aaime: Sorry? Doesnt' feature.getType.getAttribute(x) return a descriptor? (18.41.00) jdeolive: ie, in cases where minOccurs > 1 (18.41.21) aaime: I'm not speaking about removing descriptors here (18.41.35) aaime: Let's have them, but only in the PropertyType hierarchy (18.42.16) jdeolive: thinking... (18.42.50) jgarnett: hi? (18.42.57) aaime: Hi :) (18.43.06) jgarnett: I am nostly here aaime bug it was a late night so I am not sure how much help I will be (18.43.23) jgarnett: andrea you are asking about list? (18.43.29) jdeolive: jgarnett we kind of really need for your feedback on this one (18.43.35) aaime: jdeolive is (18.43.40) jdeolive: its like the biggest issue (18.43.40) jgarnett: the idea is that an attribute may appear several times ... in the list or collection (18.43.50) jgarnett: so you cannot make an assumption about order (18.44.11) aaime: I don't understand... guess I need an example (18.44.11) jgarnett: yawn; guys I am really not "smart" today so I am not going to be much help. (18.44.21) jdeolive: aaime, let me write you a code example? (18.44.30) aaime: ok, no problem, it's not like we have to do this today (18.44.34) aaime: sure (18.44.35) jgarnett: yes; I have started a document by way of answering your review (18.44.38) jgarnett: and most of it is examples. (18.45.17) jgarnett: andrea the easy one is a gps track where the Track feature has a bunch of Location and Time attribtues (18.45.21) jgarnett: and they repeat (18.45.49) jgarnett: if you ask for the location (18.45.53) aaime: why could not this be modeled as a List of a simpler type? (18.46.06) jgarnett: it could (18.46.13) jgarnett: it was a design decision made two years ago (18.46.16) jgarnett: the reason why (18.46.37) jgarnett: is so you can read through all the attributes (18.46.47) jgarnett: and see <point><time> <point><time> etc... (18.47.05) jgarnett: we would like (as object oriented bigots) (18.47.17) aaime: so the feature type is really a sequence of sequences (xml wise)? (18.47.22) jgarnett: people to make a single thing for <measure><point><time></measure> (18.47.26) jgarnett: and have them repeat that (18.47.29) jgarnett: but life is not so cool. (18.47.51) jgarnett: heh; see above about me not being awake; sorry if I am not explaining well. (18.48.08) jgarnett: basically we would like to represent the feature as it is provided to us (18.48.33) jgarnett: it is up to the application if order is important or not .. (18.48.41) jgarnett: we "don't need" to force them one way or the other (18.48.47) jgarnett: just as long as we can do our xpath queries. (18.49.01) aaime: I hope you understand that this may have the side effect of pumping up significantly memory requirements, garbage collection times, and thus making perf go downt he drain (18.49.18) jgarnett: it is not my requirement; you should talk to gabriel about it (18.49.41) jgarnett: do you remeber when we got all worked up over feature collection? and then it turned out ISO19107 did not even have that? (18.49.53) aaime: yes? (18.49.58) jgarnett: I think this may be another stupid use case where the OGC is screwing us over for GML3 or Observations and Measurements (18.50.04) jgarnett: I am going to drink some beer (18.50.14) jgarnett: and compare what we have against the ISO spec (18.50.26) aaime: k (18.50.30) jgarnett: and I expect that a lot of "fluf" can be thrown away (18.50.41) aaime: that would be most appreciated :) (18.50.49) jgarnett: but I don't want to talk about it much on email; since until I know it just ends up annoying justin (18.51.03) jgarnett: but for now; your understanding of the model is correct ... (18.51.07) jgarnett: List is fine (18.51.23) jgarnett: but understand that order is significant only in so much as the application wants it to be (18.51.39) aaime: Well, List is useful only is we can get rid of Attribute and return Object directly (18.51.43) jgarnett: it should not matter to the model. I got shot down on this one by bryce and rob at one point. (18.51.48) aaime: otherwise I too agree that Collection is a better choice (18.51.50) jgarnett: yeah; we can't do that. (18.51.57) jgarnett: best annalogy is the Class/Field/Object (18.52.05) jgarnett: or Map/Map.Entry/Value (18.52.11) jgarnett: we need something in the middle there (18.52.20) jdeolive: can i jump in here with some code? (18.52.22) jgarnett: even though it annoys me (18.52.24) aaime: Only to keep a "strange" requirement alive imho (18.52.29) jdeolive: i can wait if you are guys are in the middle ... (18.52.42) aaime: Nope, please go on (18.52.46) jgarnett: sure; although punting it on the wiki and talking about the page would be better (incase I cannot keep up with you guys today) (18.53.05) jdeolive: so andrea, if i understand you correctly you are asking why cant we do this (18.53.06) jdeolive: http://rafb.net/p/L62jAE73.html (18.53.08) sigq: Title: Nopaste - No description (at rafb.net) (18.53.18) aaime: right (18.53.32) jgarnett: yeah (18.53.35) jgarnett: no way we should be able to do that (18.53.36) jdeolive: ok, consider a complex type with one element ( ie one descriptor) (18.53.41) jdeolive: but has maxOccurs = 3 (18.53.48) jdeolive: f.getAttributes().size can be up to 3 (18.53.53) jgarnett: andrea - the other "driver" for this one (the reason I did not freak out when forced in this direction) (18.54.00) jdeolive: but f.getType().getAttributes() will always be of size 1 (18.54.01) jgarnett: was I want to be able to represent invalid data. (18.54.07) jgarnett: (that does not agree with the schema) (18.54.33) jdeolive: ie, the relationship from attribute to attribute descriptor is not one to one (18.54.34) aaime: jdeolive, why can't the model return a single object, a collection with three values inside instead? (18.54.51) jgarnett: because the three values have no meaning (18.54.56) jdeolive: because thats not really modelling xml (18.54.57) jgarnett: it is like Map.values() (18.55.04) jgarnett: you lost your keys (18.55.15) jdeolive: it would be like a xml schema description with the element "foo" 3 times (18.55.21) aaime: Ok, now I understand. We're doomed. (18.55.29) jdeolive: instead saying foo once, with maxOccurs = 3 (18.55.40) aaime: hopefully hardware will match our slowdowns in a year or so (18.55.40) jgarnett: aaime question; since I have to explain this point again and again (18.55.53) jgarnett: what is easier to under stand Class analogy or Map analogy? (18.56.00) jdeolive: aaime, about performance (18.56.08) aaime: It's just nonsense from an OO standpoint (18.56.17) jdeolive: i understand, this represents a signicant performance risk (18.56.21) aaime: there is no way you'll explain that to a java developer (18.56.26) jgarnett: fair enough; this is a geographic information model (18.56.29) aaime: do the xml example instead (18.56.32) jgarnett: not an computer science information model. (18.56.58) jgarnett: So I better go with the Map analogy; so they think of it as data. Thanks that will help. (18.57.02) jdeolive: i am going to wait in order to continue on... (18.57.04) aaime: No no (18.57.10) aaime: you better use xml, really (18.57.22) jgarnett: okay; that is also a data model. (18.57.29) aaime: map to represent data is considered the crudest, silliest way to represent data (18.57.31) jgarnett: justin I will shut up now; thanks for the chat guys. (18.57.49) aaime: jdeolive, please go on (18.58.04) jdeolive: ok, the first thign i want to say is this (18.58.34) jdeolive: i agree 1000% that this has the potential to kill performance (18.58.49) jdeolive: but there are ways to get around that (18.59.23) jdeolive: for instance (18.59.39) jdeolive: one of the things jody did back was implement a "FastSimpleFeature" (18.59.49) jdeolive: which did not use Attribute (18.59.53) jdeolive: only when it was asked for (19.00.01) jdeolive: so that if you go through the SimleFeature interface (19.00.06) jdeolive: you get the same performance we have today (19.00.06) aaime: Well, that was the frist thing I thought about it :) (19.00.12) jdeolive: just straight into an array of objects (19.00.20) aaime: The issue is that we miss a middle ground imho (19.00.25) aaime: Simple is too simple (19.00.38) aaime: there is no way to represent a sane OO model and get good performance anyways (19.00.57) aaime: I mean, if you're doing sane modelling, you should get sane performance as well (19.01.17) jdeolive: i agree (19.01.19) aaime: if you're doing the insanity above then yes, you deserve the bloody slowdown (19.01.51) aaime: but why make people pay when the have a simple Invoice-InvoiceLine association? :) (19.02.30) jgarnett: So andrea the ArrayFeature I implemented kept everything internally in arrays; and only produced the descriptor stuff on the fly as needed. That is why the SimpleFeature interface has the getAttribute( index ) and getAttribute( name ) methods (19.02.34) jdeolive: like 1-1 you mean? (19.02.37) jgarnett: to avoid paying the piper if your data is simple. (19.02.43) aaime: Maybe we should have a SaneFeature subclass between Feature and SimpleFeature ;-p (19.02.52) aaime: no no, one to many (19.02.57) aaime: but still sane :) (19.03.03) aaime: modelled as an association (19.03.20) aaime: feature having one single attribute, that is a collection (19.04.06) aaime: I mean, the most common case of complex feature is just an object composition with no xml-ness (19.05.41) jgarnett: yes; and I think when I read ISO (or actually pay attention to GML) that will be the only case. (19.06.27) aaime: It seems to me that the crux here is trying to model features as the xml looks, instead as of how the xml schema looks (19.06.32) jgarnett: I think I see what you are saying; you would like a stepping stone; that is like SimpleFeature but allows for association as well. (19.06.51) aaime: (the xml schema will have the sequence or choice declaration in between the element and its sub elements no?) (19.07.48) aaime: Yep, if it's not possible to avoid Attribute around, I would like to avoid paying the semantic and resource overhed only if it's really needed (19.08.25) jgarnett: having a hard time keeping up; how are you doing justin? (19.08.36) jgarnett: (I was looking at http://commons.apache.org/collections/api-release/org/apache/commons/collections/map/MultiValueMap.html - this is the many values for the same key idea) (19.08.37) sigq: Title: MultiValueMap (Collections 3.2 API) (at commons.apache.org) (19.09.26) aaime: Jody, I keep on thinking that's a really exotic requirement and that a good engineer would find alternate solutions that have more respect for the poor machine he's using :-p (19.10.06) jgarnett: well there is one way to find out; we only have to care about two slap happy schema fiends (19.10.25) jgarnett: we can go over gabriels sample data, which he beat out of RobA over the course of months. (19.10.32) aaime: (one of my teachers at uni described engineering as the "art of the compromise". I liked that) (19.10.49) jgarnett: I actually think that looking at their two unsuppoerted modules and reviewing the test cases so we can see in code what we are up against. (19.11.07) aaime: Guys, what about dumping this chat to the maling list and taking a break? (19.11.17) jdeolive: sure aaime (19.11.20) aaime: Maybe asking rob and gabriel for feedback? (19.11.27) jgarnett: yeah; my problem is we keep forcing the problem; can we actually compromise on this one and still meet the requirements andrea? (19.11.30) jgarnett: If so where? (19.11.33) jdeolive: actually what i need to do is go through that thread and resummarize (19.11.37) jdeolive: its getting a bit out of control (19.11.38) jgarnett: yeah; breakout IRC would be good. ---------------------------------------------------------------------------- Cheers Andrea ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Geotools-devel mailing list Geotools-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
