Python stdlib code that looks odd

[Peter Otten]

Chris Angelico wrote:

> I'm poking around in the stdlib, and Lib/mailbox.py has the following
> inside class Mailbox:
> 
>     def update(self, arg=None):
>         """Change the messages that correspond to certain keys."""
>         if hasattr(arg, 'iteritems'):
>             source = arg.items()
>         elif hasattr(arg, 'items'):
>             source = arg.items()
>         else:
>             source = arg
>         bad_key = False
>         for key, message in source:
>             ... use key/message ...
> 
> Looks odd to check if it has iteritems and then use items. Presumably
> this will catch a Python 2 dictionary, and take its items as a list
> rather than an iterator; but since the only thing it does with source
> is iterate over it, would it be better done as iteritems? 

Remember that you are looking at Python 3 code here where items() is the new 
iteritems().

> Mainly, it
> just looks really weird to check for one thing and then call on
> another, especially as it then checks for the other thing in the next
> line.

Someone mechanically removed occurences of iteritems() from the code and 
ended up being either too aggressive as the Mailbox class still has an 
iteritems() method, so mb1.update(mb2) could avoid building a list, or too 
conservative as Mailbox.iterXXX could be removed, and .XXX() turned into a 
view following the example of the dict class.

If nobody has complained until now (you get an error only for objects with 
an iteritems() but without an items() method, and those should be rare to 
non-existent) I think the path to progress is to remove the first 
alternative completely.

    def update(self, arg=None):
        """Change the messages that correspond to certain keys."""
        if hasattr(arg, 'items'):
            source = arg.items()
        else:
            source = arg
        bad_key = False
        ...

Please file a bug report.