[Xcb] [PATCH 2/2: xpyb] Fix length field handling, again.

Jamey Sharp jamey at minilop.net
Fri Mar 23 07:32:56 PDT 2012


On 3/23/12, Alex Plotnick <shrike at netaxs.com> wrote:
> -    if expr.lenfield_name != None:
> -        # This would be nicer if Request had an is_request attribute...
> -        try:
> -            has_opcode = hasattr(expr.parent.parents, "opcode")
> -        except AttributeError:
> -            has_opcode = False
> -        if has_opcode:
> -            return expr.lenfield_name
> -        else:
> -            return 'self.%s' % expr.lenfield_name
> +    if expr.lenfield_name is not None:
> +        for grandparent in expr.parent.parents:
> +            # This would be nicer if Request had an is_request attribute...
> +            if hasattr(grandparent, "opcode"):
> +                return expr.lenfield_name
> +        return 'self.%s' % expr.lenfield_name

Are you guaranteed that expr.parent and expr.parent.parents both
exist? The check for AttributeError before can only have been guarding
those field accesses, since hasattr won't throw that, right?

I'm not an xpyb user so I should probably stay out of the question,
but I don't see why you shouldn't use isinstance here. It seems more
clear to me, even if it's officially less Pythonic.

Jamey


More information about the Xcb mailing list