[Zope-Coders] SimpleItem.getId busted

[Paul Winkler]

On Thu, Nov 20, 2003 at 04:42:11PM +1100, Stuart Bishop wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> I'd like to change:
> 
>     def getId(self):
>         """Return the id of the object as a string. This method
>            should be used in preference to accessing an id attribute
>            of an object directly. The getId method is public."""
>         name=getattr(self, 'id', None)
>         if callable(name):
>             return name()
>         if name is not None:
>             return name
>         if hasattr(self, '__name__'):
>             return self.__name__
>         raise AttributeError, 'This object has no id'
> 
> to:
> 
> 	def getId(self):
>         """Return the id of the object as a string. This method
>            should be used in preference to accessing an id attribute
>            of an object directly. The getId method is public."""
> 		name = getattr(aq_base(self), 'id', None)
> 		if callable(name):
> 			return name()
> 		if name:
> 			return name
>        try:
>            return self.__name__
>        except AttributeError:
>            raise AttributeError, 'This object has no id'

looks fine except what I assume are copy/paste indendation errors.
I assume that should be:

    def getId(self):
        """Return the id of the object as a string. This method
        should be used in preference to accessing an id attribute
        of an object directly. The getId method is public."""
        name = getattr(aq_base(self), 'id', None)
        if callable(name):
            return name()
        if name:
            return name
        try:
            return self.__name__
        except AttributeError:
            raise AttributeError, 'This object has no id'

The change of hasattr / getattr to try/except is a bit gratuitous and
unrelated to the intent of your fix. I was curious if there's a performance
difference (you reminded me that I've always wondered about this),
so I did a bit of quick and dirty benchmarking. In python 2.1.3 and 2.2.3, 
try/except is about 50% faster when the try succeeds, but 50% slower 
when the exception is raised.  Six of one, half a dozen of the other...

> This makes the getId() method actually work for PageTemplateFile's,
> and all the methods that rely on it like absolute_url().
> It is sort-of backward compatible, as people doing
> string:${template/absolute_url}/myid to work around this bug will
> still get valid links thanks to aquisition, but it aint pretty.
> Any objections to me fixing the HEAD?
> 

That seems better than forcing people to discover the workaround for themselves.
+1.

-- 

Paul Winkler
http://www.slinkp.com
Look! Up in the sky! It's HYPE SCHIZOID PROFESSOR!
(random hero from isometric.spaceninja.com)