Re: CayenneDataObject#setDataContext()

From: Andrus Adamchik (andru..bjectstyle.org)
Date: Tue Sep 27 2005 - 21:44:14 EDT

  • Next message: Kevin Menard: "Re: CayenneDataObject#setDataContext()"

    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 - 21:44:17 EDT