Hi Ari,Andrus:
2009/6/15 Aristedes Maniatis <ar..aniatis.org>
> On 15/6/09 6:12 PM, Andrus Adamchik wrote:
>
>>
>> * DataMapElement ... While the name itself sounds ok, this implies that
>> the DataMap itself can't be a DataMapElement. In 3.0 we do have some
>> simple code generation capabilities for DataMap, so adding properties to
>> the DataMap seems appropriate. So maybe come back to the
>> idea of MappingObject? (also see the next point).
>>
>
> 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.
>
>
>
> * Making AbstractQuery extend from DataMapElement may or may not be ok,
>> but note that not all queries that can be mapped extend AbstractQuery.
>>
>
> Ah, why not? Shouldn't we have a rigorous class hierarchy so that common
> code can be kept in superclasses where they belong?
>
>
> In fact I'd like to get rid of this inheritance going forward... So
>> looks like the whole DataMapElement/MappingObject should
>> be an interface anyways.
>>
>
> 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. 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.
>
>
>
> * DataMap: private List<String> propertyKeyList - I don't understand
>> this one, and it is encoded in the XML. Before we add that to the
>> schema, could you please explain why it should be there?
>>
>
> I think I see what Jack might have done there. That looks like it was used
> for the CM and should be removed. I'll take a look before it is committed.
It is userd for the CM. New added "PropertyPanel" in CM needs a Property Key
List of the DataMap when "PropertyPanel" is initialized. Before discuss the
CM, ti should be removed. The old patch had no this change.
>
>
>
> * Patch for the schema includes the entire file. It is not clear what
>> was changed there.
>>
>
> Most of it :-). The indenting changed because we used XSD inheritance to
> describe the way the property can be attached to the parent class, matching
> the Java hierarchy.
>
>
> * 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.
>
>
> 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.
>
>
> * Property.getHolder() is not used anywhere. Let's not add API we don't
>> need. 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.
>>
>
> Sure.
>
> I also have some notes on the Modeler, but I suggest we settle on the
>> core framework approach first, and do incremental patches. It is much
>> easier to review and discuss things in small manageable pieces.
>>
>
> Fair enough. If Jack is still around he might like to do the tidying up,
> otherwise I'll look after it.
>
>
> Ari Maniatis
>
This archive was generated by hypermail 2.0.0 : Mon Jun 15 2009 - 05:35:17 EDT