On Thu, 21 Aug 2008, Frank Niessink wrote: > 2008/8/21 Jerome Laheurte <[EMAIL PROTECTED]>: >> Actually, I ran trunk for half a day by replacing the >> >> children=[child.copy() for child in self.__children] >> >> with >> >> children=[self.__children[:]] > > Without problems I assume, since you are not mentioning any problems?
Indeed, but since I don't use undo very often, it doesn't mean much. I just checked that the task editor showed up OK after the category editing/undo stuff. There may be side effects when doing more complicated stuff. > I'm not entirely sure, but I may have been trying to have copy() use > __getstate__() because of the duplication between __getstate__() and > copy(). I must have decided that that wasn't really possible but not > changed things back again in good order. > > I may have seen this (example from patterns.Composite): > def copy(self, *args, **kwargs): > kwargs['parent'] = self.parent() > kwargs['children'] = [child.copy() for child in self.children()] > return self.__class__(*args, **kwargs) > > and thought to myself: hey, I can use __getstate__ to get the > necessary information and then simply have copy look like this > def copy(self): > return self.__class__(**self.__getstate__()) > > Actually, this is what currently happens in domain.base.Object: > def copy(self): > state = self.__getstate__() > del state['id'] # Don't copy the id > return self.__class__(**state) So why not use the standard library 'copy' module, which does just that, AFAIK ? > But that only works because all base.Object attributes are immutable! Indeed. But CompositeObject attributes aren't, thus the problem. > So, I guess we need to make some decisions on how state manipulation > and copying should work (a protocol so to say :-). How about adding a > boolean parameter to __getstate__ called 'deepcopy' that returns a > deep copy of the state if True and a shallow copy otherwise, and then > simply call the method from base.Object.copy? Mmmh, I'm not keen on adding an extra keyword argument to a special method used by the standard library (I think __getstate__ and __setstate__ are used by both copy and pickle). There's something that bugs me there: a method that is supposed to return a state, independent from any application-specific class, actually returns something that include instances of domain objects. I think that instead of including a list of children copies, it should include a list of children states. As I said earlier, this will need special handling for deletion/creation of children, but I think it's worth it. Cheers Jérôme
