On 6/17/10 14:53 , Chris McDonough wrote:
> On Thu, 2010-06-17 at 11:28 +0200, Wichert Akkerman wrote:
>> I noticed something odd in repoze.folder: __setitem__ does not allow you
>> to replace an existing item. This is a result from __setitem__ being an
>> alias for add(). Is that a deliberate design decision? If not I'ld like
>> to change it to allow replacing items.
>
> The folder API isn't meant to exactly mirror the dictionary API, so I
> don't consider this odd. Most CMS-ish UI operations that call for
> adding a new item also call for aborting if an item by that name already
> exists, so we default to that behavior.
>
> But it probably doesn't matter much. As long as the deletion sends (or
> doesn't) the proper ObjectRemoved, etc events, I'd be sort of +0 on
> being able to replace an existing item. I think this would imply
> changing .add rather than changing __setitem__. Please read the
> docstrings for .add and .remove before changing much; we need to retain
> the ability to add and remove an item with and without sending events.
> If we change things so doing an addition replaces, and if someone adds a
> new item that already exists, .add should call .remove with the
> send_events argument the same value as what was passed to .add.
My suggestion would be to make __setitem__ always do remove and add with
events. If someone needs more control they can explicitly call add and
remove directly. Allowing add() to replace items feels counterintuitive
to me. To put this in code my proposal would be:
def __setitem__(self, key, item):
"""Set a child item, optionally replacing an existing item with
the same key. This will always fire object events. If you want to
block events use add() and remove() directly.
"""
if key in self:
self.remove(key)
self.add(key, item)
Wichert.
_______________________________________________
Repoze-dev mailing list
Repoze-dev@lists.repoze.org
http://lists.repoze.org/listinfo/repoze-dev
