On 10/20/07 4:36 AM, "Andrus Adamchik" <andru..bjectstyle.org> wrote:
>
> On Oct 19, 2007, at 10:58 PM, Kevin Menard wrote:
>
> I think actually the suggested terse syntax is more confusing on a
> number of levels:
>
> * normally a user would assume that after "a.setSomething(b)",
> "b.getSomethingElse() == a" (i.e. the reverse relationship is set),
> which would not be the case as reverse is set for the clone of "b".
> * and on a higher level it would obscure the fact that cross-context
> relationships are not possible by design.
Fair enough, but I've seen the problem play out in reverse as well. People
keep working with the original object, not the cloned. This gets strange as
that code starts getting distributed about. E.g., you have one method to
modify some type A and get it into a committed state. Another method where
you associate that A with B and maybe make some changes to it, but need not
commit them quite yet. Now you need to send that A out to every place that
you accessed the original A from.
Apologies if that seemed confusing. I can try to use specific cases if need
be. Suffice it to say, this is something that junior developers here have
done several times and it's not entirely fun to work out. Logically,
everything works the way it should. It's not until you consider how Cayenne
works that the problem becomes clear.
> I.e. I think explicit is good in this case. Also with JDK 1.5 switch
> we can remove the need for casts in many situations, making current
> approach less verbose.
Agreed. Although, I think having to grab the context and then calling
localObject() on that is still long-winded. If that be the case, I'd rather
see a new form of localObject() that takes a single Persistent and extracts
the object ID accordingly. I don't think I've ever seen the case where the
two parameters did not match in some way (save for using null as the second
one). It could be less verbose to move localObject() up to a utility method
of CDO itself as well.
>
>> 2) Loss of transient values.
>>
>> This could probably be addressed reflectively.
>
> True. Also JPA introduces explicit "transient" attributes - something
> we can use to address this issue.
I'll have to read about that. Is this something better addressed for JPA
first or for classic Cayenne first?
>
>> 3) Growth of object store.
>>
>> This is trickier. Ideally, if I call setSomething(a) with 50
>> different
>> instances of a, the object store would only have the latest a,
>> since that's
>> the only one that's going to be committed. What you have instead
>> is 50
>> different instances. With caching, I don't think DB access is the
>> issue,
>> but you will have an unbounded memory issue.
>
> I wouldn't worry about that in 3.0 - ObjectStore is using weak
> references now, so if you don't have a hard reference in the user
> code, the object will be gc'd as needed.
Okay. That's good to know. There is still a bit of a problem though,
because any changes made to any of the other 49 replaced objects objects
will still be committed and still potentially cause validation failures.
Maybe this should be expected, but what you have is an object that really
isn't attached to the graph at all any longer that will be committed. For
me, that creates for some confusing behavior. Debugging any validation
failures at that point is quite difficult, because your object graph could
validate fine but some cruft left in the object store is causing the
problem. If no one else views that as problematic though, I'll humbly
resign my stance on the matter.
>
>> By hiding this in a setter, it may be possible to unregister
>> the old
>> object first, thereby limiting the growth.
>
> Not a good idea. Consider this:
>
> 1: BType blocal = (BType) a.getObjectContext().localObject
> (b.getObjectId(), b);
> 2: a.setSomething(blocal );
> 3: a.setSomething( (BType) a.getObjectContext().localObject
> (b1.getObjectId(), b1));
> 4. blocal.doSomething();
I guess I don't see that being distinctly different from the following
non-Cayennish code:
1) BType b = a.getSomeB();
2) a.setSomeB(b1);
3) BType b1 = a.getSomeB();
4) b.doSomething();
What I do view as being problematic is if line 2 did not exist and the "b"
and "b1" are not the same object at all. This is sort of the situation we
have today.
In either case here, the call in line 2 sets the expectation that the
reference in line 1 is no longer overly useful (I'm sure someone can think
of a case where it is, but when working with a's latest b, it is not). One
could even draw a nice state diagram showing how this is the case and this
is not particular to Cayenne.
> "blocal" will be kicked out of the a's context in line 3, but the
> user already referenced this object in line 1, so when he later
> accesses it on line 4, it is no longer persistent. As I mentioned
> above there's no problem anymore with unused object accumulation, and
> generally trying to second-guess Cayenne on object graph management
> may cause undesired side effects.
Unfortunately, this doesn't appear to be the case. blocal will commit just
fine, which creates more problems to me than if the commit would fail
altogether.
We've gone back and forth on this a few times now. As long as the
conversation is still productive, I don't mind following it. Unfortunately,
I don't have as great an understanding of the internals as you do, so if
we're just rehashing old ground, we can agree to disagree. What I'd just
hate to see is if we continue to do things the way we have been out of force
of habit, rather than because it's the best way to do it. I see a lot of
room for improvement and based on graph theory, can see how it could be
solved. I haven't ascertained from the Cayenne code how disruptive a change
it would be, however.
-- Kevin
This archive was generated by hypermail 2.0.0 : Sat Oct 20 2007 - 09:41:26 EDT