Thanks for the comments. Obviously, you're much more familiar with the API
than me.
Your solution sounds like a good one to me. At the very least, I'd like to
see updated JavaDoc comments for methods that are not intended to be called
by the end-user directly. This really would go a long way in helping people
like me out, I think.
So, is this something you'd be willing to do, or should I invest some time
into it? If the former, I'll move back to the selectable PK generators I
was playing with earlier.
Thanks,
Kevin
On 9/27/05 9:44 PM, "Andrus Adamchik" <andru..bjectstyle.org> wrote:
> Kevin,
>
> I am not psyched about such patch at all. This makes too many
> assumptions...
>
> This method (as well as setObjectId() as well as most of the
> DataObject API) was designed to be accessed by framework, so yes, we
> are exposing too much to the users. I'd be more in favor of
> deprecating those methods all together and use reflection to set the
> corresponding field directly. I'd probably go this way with new
> Persistent interface, and we can do the same with DO and CDO.
>
> Re: patch code itself - "getObjectStore().getObjects()" is expensive
> as it returns array by copy. "contains" is also expensive. It will
> have O(N) performance, unacceptable in cases with more than a few
> registered objects. ObjectStore.getObject(ObjectId) seems more sensible.
>
> Andrus
>
>
> On Sep 27, 2005, at 8:59 PM, Kevin Menard wrote:
>
>> On 9/14/05 10:00 AM, "Cris Daniluk" <cris.danilu..mail.com> wrote:
>>
>>
>>> When setDataContext() is called, the DataContext should know its
>>> going
>>> to be holding the CDO. Couldn't setDataContext check the object store
>>> and see if the CDO is already in there. Then it could throw an
>>> exception if not, preventing the confusing situation...
>>>
>>> This may not work, especially with serialization, etc.. just a
>>> random idea.
>>>
>>
>> I've prepared a patch around this idea. Any comments on it would
>> be greatly
>> appreciated. The new setDataContext() looks like:
>>
>> public void setDataContext(DataContext dataContext) {
>> if (dataContext == null) {
>> this.persistenceState = PersistenceState.TRANSIENT;
>> }
>>
>> else {
>> if ((null == dataContext.getObjectStore()) ||
>>
>> (!dataContext.getObjectStore().getObjects().contains
>> (this))) {
>>
>> throw new CayenneRuntimeException("Object is not registered
>> with
>> data context. Register the object prior to calling this method.");
>> }
>> }
>>
>> this.dataContext = dataContext;
>> }
>>
>>
>> Apologies for the poor formatting.
>>
>> This change breaks a lot of tests though. I don't have a problem
>> going
>> through and fixing them, but I want to make sure this is the right
>> way to be
>> going before I waste my time.
>>
>> Thanks,
>> Kevin
>
This archive was generated by hypermail 2.0.0 : Tue Sep 27 2005 - 22:00:35 EDT