Hi Jack, Ari

Finally had some time to look at the patch. It is moving in the right direction, but there are still some issues we need to address:

* 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).

* Making AbstractQuery extend from DataMapElement may or may not be ok, but note that not all queries that can be mapped extend AbstractQuery. 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.

* 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?

* Patch for the schema includes the entire file. It is not clear what was changed there.

* 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. Also if there is a notion of properties order in the element (is there?), I guess the property should be stored in the list. If there's no notion of

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

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.

Andrus






On Jun 5, 2009, at 5:48 PM, Andrus Adamchik wrote:

[Let's take this to a separate thread.]

On Jun 5, 2009, at 5:33 PM, Aristedes Maniatis wrote:

Back on dev topics, Jack has finished his secondment at ish now and posted his DataMapElement.Property implementation. I'll do some cleanup on it (mostly naming and style) and some commenting and commit it unless anyone has something to say.


Could you please let it sit in Jira for some time. I'd like to review it, but can't do it right this second.

Andrus




Reply via email to