[Zope3-dev] Re: [Fwd: [Z3d] 562/ 3 Reject "ObjectCopier must copy not reference the object"]

[Philipp von Weitershausen]

Egon Frerich wrote:
>>>> On my PC I've changed zope\copypastemove\__init__.py (lines 396 - 403) from
>>>>
>>>>>>         checkObject(target, new_name, obj)
>>>>>>
>>>>>>         chooser = INameChooser(target)
>>>>>>         new_name = chooser.chooseName(new_name, obj)
>>>>>>
>>>>>>         copy = locationCopy(obj)
>>>>>>         copy.__parent__ = copy.__name__ = None
>>>>>>         notify(ObjectCopiedEvent(copy, obj))
>>>> to
>>>>
>>>>>>         copy = locationCopy(obj)
>>>>>>         copy.__parent__ = copy.__name__ = None
>>>>>>
>>>>>>         checkObject(target, new_name, copy)
>>>>>>
>>>>>>         chooser = INameChooser(target)
>>>>>>         new_name = chooser.chooseName(new_name, copy)
>>>>>>
>>>>>>         notify(ObjectCopiedEvent(copy, obj))
>>>> The tests passed. The problem 562 is gone.
>>>>
>>>> Can somebody review this change?
>>> Ever heard of a (unified) diff? Makes it easier to review code changes.
> 
> Yes - I can read the diff between the basis and my working copy with
> TortoiseMerge. But if you want to see the diff I have to upload the
> change to the repository. Before uploading this change somebody should
> look at it because this central module should not make problems.

I'm sure you must be able to review a diff before you commit... If not,
drop Tortoise and get something that lets you do this. How else would
you be able to submit patches?

>>>> And please have another look at my NameChooser function and give me an
>>>> explanation why you think "his logic in the namechooser seems bogus too"
>>> The way it modifies the object and call itself again etc. just looks
>>> bogus. I don't think name chooser should be able to modify the object
>>> they're choosing a name for. After all, they're just choosing a name.
>>>
>>> I think you really want your own object copier if you have a requirement
>>> that obj.name should be unique throught the system. Again, see the issue
>>> transcript.
>>>
>>> Philipp
>>>
> @Philipp:
> In your book you write (page 245): "Why not synchronize these names and
> make recipe's names automatically their names inside the container?" I
> would say this is a normal use case. Afterwards you present your
> solution. But with your solution the copy function is not possible.

That's true. That's why I would suggest a custom IObjectCopier.

> My opinion is: if we have to synchronize the names then it should be
> done in the NameChooser. One place - one responsibility.

Right. The responsibility is choosing names. Not setting attribute values.

> If you want to synchronize names then you have to change the value
> *in* the object too if the container cannot include the object with
> the supplied name.

I disagree. But, this is your code, so you can do whatever you see fit.
I would simply advise against that.

Philipp