Leonardo Rochael Almeida wrote:
Please revert this change. You've changed the semantics of this
statement and it will now mask errors in traversing to the path.
- obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid))
- if not obj:
+ obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None)
+ if obj is None:
This is bad...
I agree with you the semantics have changed, but I argue that they've
changed to what they were intended to be in the first place :-)
This argument has been had many times before, please google zope-dev's
archives for the numerous discussions on this and then check the current
This is the full method before my changes:
def getobject(self, rid, REQUEST=None):
"""Return a cataloged object given a 'data_record_id_'
obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid))
if not obj:
if REQUEST is None:
obj = self.resolve_url(self.getpath(rid), REQUEST)
This code is just plain rubbish, there's no single bug here.
I'd like someone to give a use case where unrestrictedTraverse fails and
yet where resolve_url works.
Traversing with a default argument will only mask traversal errors, not
any other errors that might happen during traversing.
Sure it will, see previous discussions...
In the version with my changes, but with the old unrestrictedTraverse
call, the "if obj is None" would only ever be reached in the
pathological situation where the attribute at the end of the path is
None. Which means the ".resolve_url(self.getpath(rid), REQUEST)" would
hardly ever be called at all.
Correct. .resolve_url(self.getpath(rid), REQUEST) hardly ever gets called.
A) Keep my changes, but use a marker "object()" instead of None for the
"default" parameter of the .unrestrictedTraverse() call.
B) Reverse my changes, but also remove the "if not obj" block, which is
buggy whichever way you look at it with the old unrestrictedTraverse
Which one should it be?
I'd go for B.
Simplistix - Content Management, Zope & Python Consulting
Zope-Dev maillist - Zope-Dev@zope.org
** No cross posts or HTML encoding! **
(Related lists -