Florent Guillaume wrote:
2. is necessary for backward compatibility. *all* the previous implementations of getObject returned None in case of problems.

This is the only bit I'm asking about, I accept that I'm in the insane minority on the other point ;-)

Just because that's what it did before doesn't mean we should leave it like that. I can see absolutely no benefit in returning None over raising a specific error.

Also, the original behaviour of getObject, prior to Casey's drastic and unexpected switch to restrictedTraverse(path,None) was to raise Unauthorized, NOT to return None.

I first became aware of Casey's changes when I upgraded a production Zope instance and started getting loads of random attribute errors where I'd happilly been dealing with the Unauthorized's before.

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.

Now, the other problem I have with this fix is no tests were checked in for this. We should have tests that verify this behaves as it should, since those tests are likely to be the only formal documentation this issue ever receives ;-)

Would anyone object if I wrote tests and changed the implementation to raise exceptions, including Unauthorized, instead of returning None?



Simplistix - Content Management, Zope & Python Consulting
- http://www.simplistix.co.uk
Zope-Dev maillist - Zope-Dev@zope.org
** No cross posts or HTML encoding! **
(Related lists - http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

Reply via email to