Hi there,
On 27/03/2007, at 1:36 AM, Andrus Adamchik wrote:
> Splitting my comment on Lachlan's patch into smaller manageable
> pieces. See comments on ObjectContext inline...
>
>
>> Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
>> apache/cayenne/BaseContext.java
>> ===================================================================
>
>
>> + public Persistent localObject(ObjectId id, Object prototype) {
>> +
>> + if (id == null) {
>> + throw new IllegalArgumentException("Null ObjectId");
>> + }
>> +
>
> +1, but note that there are some slight differences between
> DataContext and CayenneContext implementation. Don't recall all the
> details, but it would be nice to diff them and see what we can do
> about them.
There was no difference actually. The only diff was a comment in
CayenneContext saying that this was a direct copy from DataContext
and a comment in DataContext instructing people to remember to copy
the implementation to CayenneContext and a TODO saying we need to
find inheritance somehow... so it seemed the natural thing to do the
todo.
>> Index: framework/cayenne-jdk1.4-unpublished/src/main/java/org/
>> apache/cayenne/ObjectContext.java
>> ===================================================================
>
>
>> /**
>> + * Creates a nested context
>> + */
>> + ObjectContext createNestedContext();
>> +
>> + /**
>> * Returns EntityResolver that stores all mapping information
>> accessible by this
>> * ObjectContext.
>> */
>
> -1: I think we should start with implementation of nested contexts
> on the client, before declaring it in the interface (i.e. before
> announcing it to the users). Also I've been doing some work as a
> part of CAY-739. So maybe this method will be moved to the *Utils
> class all together.
Fair enough. This is an area of particular interest to me (and ish in
general). So I'd be keen to help with this where I can.
>> /**
>> + * Returns an object local to this ObjectContext and matching
>> the ObjectId.
>> + * <p>
>> + * Unlike ..ink #localObject(ObjectId, Object)}, this method
>> will only do "mapping"
>> + * (i.e. finding an object with the same id in this context).
>> + * </p>
>> + */
>> + Persistent localObject(ObjectId id);
>
> ...
>
>> /**
>> + * Returns an object local to this ObjectContext from the
>> given object.
>> + * <p>
>> + * This is essentially a convenience method for ..ink
>> #localObject(ObjectId)}.
>> + * </p>
>> + */
>> + Persistent localObject(Persistent aPersistentObject);
>> +
>
> -1: IMO the interface must be as lean as possible to simplify alt.
> implementations and decorators. So convenience (in other words
> derived from other) methods should preferably be defined outside
> the interface. I think this is such case.
Would localObject(Persistent) then be better as an interface than
localObject(ObjectId,Object)? The latter is already possible via
DataObjectUtils.objectForPK(ObjectContext,ObjectId). But the former
seems more generally useful.
>>.. -141,6 +163,14 @@
>> void commitChangesToParent();
>>
>> /**
>> + * Determines if there are any changes in this context that
>> can be flushed to the
>> + * parent DataChannel. i.e., determines if ..ink
>> #commitChangesToParent()} would
>> + * flush any changes to the parent DataChannel or, likewise, if
>> + * ..ink #commitChanges()} has changes to cascade through
>> the stack to the database.
>> + */
>> + boolean hasChanges();
>> +
>> + /**
>> * Resets all uncommitted changes made to the objects in this
>> ObjectContext, cascading
>> * rollback operation all the way through the stack.
>> */
>
> +1: while it is somewhat redundant, it should allow implementors to
> optimize analysis of graph changes.
Great. It certainly makes the user's code a bit cleaner ;-)
with regards,
--Lachlan Deck
This archive was generated by hypermail 2.0.0 : Mon Mar 26 2007 - 16:44:48 EDT