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.
Andrus
