On 16/12/2007, at 11:28 PM, Andrus Adamchik wrote:
> On Dec 16, 2007, at 1:49 PM, Aristedes Maniatis wrote:
>
>> Current EmbeddedAttribute extends Attribute, but I wonder whether
>> it should be extending ObjAttribute.
>
> I don't think it meets a criteria for inheritance, not with
> ObjAttribute superclass in its current form, as EmbeddedAttribute
> has no dbAtributePath.
Strictly you are right, but perhaps a null dbAttributePath is the
easiest solution. Is it very different from dbEntityName in
ObjEntities with single table inheritance?
>> If it were to do so, lots of nice simplification will be had by
>> having getAttributes return Collection<ObjAttribute> rather than
>> Collection<Attribute> (and the equivalent for getRelationships and
>> also the same for DbEntity).
>
> Actually there is a bigger problem that prevents us from making
> these simplifications. And that is "coincidental" common
> superclasses: Entity, Attribute, Relationship. I.e. IMO Db* and Obj*
> should not be in the same hierarchy. But changing that now with
> backwards compatibility in mind is fairly non-trivial.
And I've just spent about 3 hours now trying to figure a way to
untangle them somewhat. For example, there is getAttributes which is
defined in Entity, overridden in ObjEntity but not in DbEntity.
Sometimes is returns ObjAttributes (from ObjEntity), sometimes
DbAttributes (from DbEntity) and the resulting collections are cast to
the desired result all over Cayenne. No compile time checking at all
and if we implement generics in the simplest possible way
(Collection<Attribute>) we don't really solve that. And now that I've
just stumbled on EmbeddedAttributes in two places I realise that about
50 other places completely ignore their existence and will be trying
to cast them to ObjAttribute.
I'm working through these generics also with inheritance in mind and I
think the problem might get worse, with DbEntity needing to get a
superEntityName just like ObjEntity, and other things which will
intertwine the db and obj classes in new ways.
But my purpose in moving getAttributes to Collection<Obj/DbAttribute>
was to help unmix the two sets of classes. We could keep the hierarchy
for now and try to eliminate many of the uses of the superclasses from
the code (mostly they are not helpful or needed).
>> If not, then there appear to be places in Cayenne which assume the
>> collection just contains ObjAttributes:
>>
>> EntityMergeSupport.getMeaningfulFKs() (line 170)
>> PrefetchProcessorJointNode (line 205)
>> and more...
>
> Correct, this assumption is no longer true.
Well, I think there are lots of these cases from my look just now.
>> It does seem to me that EmbeddedAttribute should logically extend
>> ObjAttribute since it is a specialised case of one. What do you
>> think?
>
> I don't think one should inherit from another (see above). Having a
> common superclass for both may be more appropriate.
Although I agree that Entity, Attribute and Relationship are a bit
unnecessary, I don't think it serves much purpose to get rid of them
in the short term. Instead we should push down references to the
subclass wherever possible. Which is part of what I've started to do.
I am not sure of the best way to proceed, but one approach would be to
make EmbeddedAttribute a subclass of ObjAttribute, then create a new
PlainAttribute (better name?). We can deprecate the bits of
ObjAttribute which need to be pushed down into PlainAttribute and move
them over time. But the overall goal is to leave very few if any
direct references to the Attribute, Entity and Relationship
superclasses in our code.
What do you think?
Ari
-------------------------->
ish
http://www.ish.com.au
Level 1, 30 Wilson Street Newtown 2042 Australia
phone +61 2 9550 5001 fax +61 2 9550 4001
GPG fingerprint CBFB 84B4 738D 4E87 5E5C 5EFA EF6A 7D2E 3E49 102A
This archive was generated by hypermail 2.0.0 : Sun Dec 16 2007 - 09:05:18 EST