On 10/20/07 4:36 AM, "Andrus Adamchik" <[EMAIL PROTECTED]> 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
