[Zope3-dev] proposed changes to contained helper functions

[Garrett Smith]

This is a mini-proposal based on my experience in refactoring for 
containergeddon.

My overall feedback is that the geddon is a *huge* improvement in the 
maintainability of container/contained relationships. I'm delighted with 
the prospect of never seeing the word 'wrapper' in our code again :)

However, the current set of helper functions (setitem, containedEvent, 
uncontained) are very specific to containers. While this may make sense 
for most applications, I have a couple of use cases that fall outside 
the current design:

1. I need to setup parent/child (i.e. container/contained) relationships 
for object attributes:

   class Foo:
       def __init__(self):
           self.bar = Bar()  # bar is a child of a Foo object

In this case, I'm interested in setting up parent/child relationships, 
but not generating events. I would be inclined to use containedEvent for 
this case, but it creates an event and its semantics are wrong.

2. I have a class that uses a container internally to store objects, but 
is itself not a container:

   class Foo:
       def __init__(self):
           self._data = {}

       def addBar(self, name, bar):
           self._data[name] = bar  # bar is a child a Foo object

       # other bar related methods, e.g. getBars, containsBar, etc.

In this case, I'm interested in setting up parent/child relationships 
*and* generating events. I would be inclined to use setitem, but setitem 
assumes that the parent implements IContainer, which in this case false.

Below are my proposed changes:

1. Add a new helper function 'containedObject'

  def containedObject(object, parent, name=None):
      if not IContained.isImplementedBy(object):
          if ILocation.isImplementedBy(object):
              zope.interface.directlyProvides(object, IContained)
          else:
              object = ContainedProxy(object)

      object.__parent__ = parent
      object.__name__ = name

      return object

The class Foo in case 1 would be as follows:

   class Foo:
       def __init__(self):
           self.bar = containedObject(Bar(), self, 'bar')

2. Add a new helper function 'setContained':

  def setContained(parent, setf, getf, name, object):

      # Do basic name check:
      if isinstance(name, str):
          try:
              name = unicode(name)
          except UnicodeError:
              raise TypeError("name not unicode or ascii string")
      elif not isinstance(name, unicode):
          raise TypeError("name not unicode or ascii string")

      if not name:
          raise ValueError("empty names are not allowed")

      old = getf(name)
      if old is object:
          return
      if old is not None:
          raise DuplicationError(name)

      object, event = containedEvent(object, parent, name)
      setf(name, object)
      if event:
          if event.__class__ is ObjectAddedEvent:
              a = zapi.queryAdapter(object, IAddNotifiable)
              if a is not None:
                  a.addNotify(event)
          a = zapi.queryAdapter(object, IMoveNotifiable)
          if a is not None:
              a.moveNotify(event)
          publish(container, event)
          modified(container)

This would allow Foo's addBar method to be as follows:

  def addBar(self, name, bar):
      setContained(
          parent=self,
          setf=self._data.__setitem__,
          getf=self._data.get,
          name=name,
          object=bar)

This function is identical to setitem with the exception that it deals 
with a generic parent rather than a container.

This function would allow setitem to be spelled as follows:

  def setitem(container, setitemf, name, object):
      setContained(container, setitemf, container.get, name, object)

3. Rename 'container' arg in containedEvent and uncontained to 'parent'

I agree with the decision to implement the container bookkeeping 
directly in container classes rather than through adapters -- it's much 
simpler and therefore less prone to error. However, I think the current 
implementation is a bit too inflexible. These proposed changes are an 
attempt to add enough flexibility without having to resort to adaptation.

  -- Garrett