launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

> You probably want to subclass userdict instead of dict, may avoid some
> of your boilerplate.
> 
> Looks good otherwise.
> 
> -Rob

Thanks for the review.

Actually, I initially used DictMixin and that "worked" for the pages I looked at manually but tests failed because there's specific code that calls isinstance(dict) and UserDict does not subclass dict. Java is much better is this regard since there's standard collection interfaces everything can subclass from :-) Also, to my mind, now that Python supports subclassing dict directly there's little reason to continue using UserDict? Lastly, the few boilerplate methods that are there are actually necessary to make the class behave as expected when methods like values(), items() etc are called and any as yet unconstructed values need to be instantiated.

I assume then that you are happy with the change in exception type for broken links? ie LocationError instead of KeyError. The only way to fix it is to modify the zope source code.

-- 
https://code.launchpad.net/~wallyworld/launchpad/improve-menu-rendering/+merge/38222
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/improve-menu-rendering into lp:launchpad/devel.