2009/6/15 Andrus Adamchik <[email protected]> > > On Jun 15, 2009, at 11:49 AM, Aristedes Maniatis wrote: > >> I take your point, but I still think "DataMapElement" is more descriptive. >> Every class in Java is an Object, so using that word as part of the name >> conveys nothing, except that it is sort of a generic multi-purpose thing. At >> least DataMapElement is clear and to the point. And there is no reason why >> an Element can be used to describe theDataMap itself. Slightly odd, but not >> too confusing. >> > > MappingElement then? Not tied to any particular class name and has no > implied ownership. > > Shouldn't we have a rigorous class hierarchy so that common code can be >> kept in superclasses where they belong? >> >> I don't have the experience you do in creating java libraries meant to be >> used by lots of people in different ways, but I can't see how moving to >> interfaces for this one thing will help. If interfaces are to be used, then >> perhaps the ultimate goal is that every 'data map element' should be only an >> interface, allowing a user to swap in any implementation they want without >> having to inherit from our classes. >> > > I don't insist on using interface here, rather I oppose a common superclass > for .map and .query. Inheritance hierarchies can be tricky - it's way too > easy to mix together unrelated things, based on a single coincidental shared > property (no pun intended with "property API" being developed here). So my > rule of thumb is that when in doubt, don't use a common superclass for the > sake of elusive reuse of a few lines of code. > > In fact we probably don't need a common interface without code reuse: .map > and .query can handle properties separately in their own hierarchies. For > the sake of simplicity initially I would even ignore queries completely, and > concentrate on the mapping elements. > > If not, then I think this half way mixture of inheritance and interfaces is >> confusing. There is already a mixture of the two I find sometimes >> disconcerting. >> > > The current state of Cayenne is a result of evolution, not of a one time > design, so there are some rough edges for sure. > > * DataMapElement.Property inner class... Why is this an inner class? It >> >>> is exposed via public methods to the end users, so let's make it a >>> standalone class. >>> >> >> I thought inner class made sense here. The naming is convenient and >> clearer: "DataMapElement.Property" rather than "DataMapElementProperty" >> shows the fact that it is intrinsically part of DataMapElement and not >> relevant as a "Property" outside of that usage. This is just about naming >> and style, and the decision based on what 'looked' right from the >> perspective of a user seeing this class. You originally wanted just a >> Map<String,String> of properties rather than a whole class, but I thought >> that this was a way to capture more information in the future: properties >> might have other attributes such as whether they travel to the client in >> ROP. But really, they are just a slightly enhanced Map, so an inner class >> seemed to capture that idea. >> > > This is just a matter of API style consistency - in Cayenne we do not have > *public* inner classes at all. So I'd vote for > org.apache.cayenne.map.Property. > > > Also if there is a notion of properties order in the >> >>> element (is there?), I guess the property should be stored in the >>> list. Do we care about properties ordering or simply displaying them in >>> an alphabetic order is ok? >>> I'd say alphabetic is ok. Any scenarios when the order might be >>> important? >>> >> >> I don't think order is important, so the CM can display it in whatever >> order makes sense. >> > > If order is not important, and we don't have to store properties in a list > as a result, we may not even need to store Property.key inside the property. > To me a key is what a holder decided to name the property, not something > that a property itself knows about.
I think that the key is needed inside the property. Function of property may be extended in future base on the key. Now, the properties in each DataMapElement/MappingElement are stored using SortedMap, just like Entity using SortedMap to store its attributes and relationships. *Also since we have lots of DataMapElements in the DataMap, and only some will have properties, let's use lazy initialization of the map to save some memory.* You are right, it should be modified to lazy initialization. Andrus
