[Zope-dev] brain.getObject and traversal

[Chris Withers]

Florent Guillaume wrote:

> You're mistaken. The old code did:
> 
>   def getObject(self, REQUEST=None):
>         """Try to return the object for this record"""
>         try:
>             obj = self.aq_parent.unrestrictedTraverse(self.getPath())
>             if not obj:
>                 if REQUEST is None:
>                     REQUEST = self.REQUEST
>                 obj = self.aq_parent.resolve_url(self.getPath(), REQUEST)
>             return obj
>         except:
>             zLOG.LOG('CatalogBrains', zLOG.INFO, 'getObject raised an 
> error',
>                      error=sys.exc_info())
>             pass
> 
> Which effectively returns None.

Interesting. Where did that code come from?

> You probably had the unauthorized *after* the getObject, when it 
> returned to you an object you weren't actually supposed to try to access.

possible, but at least I had an unauthorized generated, rather than just 
getting a useless None.

>> So, for me, returning None is just plain evil. All it serves to do is 
>> mask an exception that's likely to be useful. If people are relying on 
>> it returning None, then it's a one line change in their code.
> 
> All robust old code had to be able to test for None, because it was 
> returned in many cases (when indexes got desynchronised, due to 
> transaction bugs for instance, or manage_beforeDelete swallowing stuff, 
> or conflict errors happening...). I know I had to add lots in my code.

Just because you had to do it in old code doesn't make it not evil. 
Returning a meaningless value that masks a whole array of possible 
distinct errors that should all be handled in different ways is mind 
numbingly stupid IMNSHO. Why you feel so passionately that this should 
be the case is beyond me. Do you like using bare try: except:s 
throughout your code too?!

> (but zope-checkins list had problems that day, I don't know why, and the 
> checkin mail never appeared).

Apologies, that'd explain it.

>> Would anyone object if I wrote tests and changed the implementation to 
>> raise exceptions, including Unauthorized, instead of returning None?
> 
> Unauthorized in getObject is out of the question, that would be new 
> behaviour.

Well, in 2.8, new behaviour is expected, right?
I really passionately believe that we should not be returnining None in 
Zope 2.8, and since 2.8 hasn't quite hit beta yet I'm very keen to see 
it fixed asap.

Any objections?

Chris

-- 
Simplistix - Content Management, Zope & Python Consulting
            - http://www.simplistix.co.uk