[Zope3-dev] Re: proposed changes to contained helper functions

[Garrett Smith]

Just a quick followup to this...I noticed one area in the existing Zope 
source that would benefit from change 1 below:

Current code in zope/app/services/servicecontainer.py, lines 45ff:

     def setSiteManager(self, sm):
         ...
         if IServiceService.isImplementedBy(sm):
             self.__sm = sm
             sm.__name__ = '++etc++site'
             sm.__parent__ = self
         else:
             raise ValueError(...)
         ...

would become:

     def setSiteManager(self, sm):
         ...
         if IServiceService.isImplementedBy(sm):
             self.__sm = containedObject(sm, self, '++etc++site')
         else:
             raise ValueError(...)
         ...

While this isn't a revolution ;-) it's the type of explicit syntax that 
I'd expect in something as central as container/contained relationships 
in Zope.

  -- Garrett


Garrett Smith wrote:

> 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