[Mesa-dev] [PATCH 22/41] glapi: gl_XML.py: convert gl_api.functionIterateByOffset to a generator

[Dylan Baker]

Quoting Ilia Mirkin (2016-04-01 08:46:19)
> IMHO this is still quite fancy and unnecessarily complicated.
> 
> How about
> 
>   temp = dict((f.offset, f) for f in self.functions_by_name.itervalues())
>   return (temp[offset] for offset in sorted(temp))
> 
> That's all that's happening there, right? This lets you not use yield
> (which is confusing to most people, as Brian points out), but still
> uses a generator to avoid creating that second list, behind the
> scenes, while staying understandable to people who understand list
> comprehensions (which, hopefully, anyone using python by now does...
> they've been around since at least Python 2.2)
> 
> [BTW, this is not a complete review of the series... just looking at
> this patch because Brian happened to reply to it.]

I knew there must be an easier way to do this, I had a different
implementation that was simpler, but didn't sort correctly, before this
one, and I wasn't really happy with this.

I don't think we actually need to care about max_offset do we? Since
that was more an implementation detail of the original function, I'll
rebase out the previous patch too.

I don't think yours is exactly right though, since it doesn't handle the
offset == -1, but that's trivial.

Something like this(?):

temp = [(f.offset, f) for f in self.functions_by_name.itervalues()
        if f.offset != -1]
return (func for _, func in sorted(temp))


I guess we could make one generator comprehension out of it, but
performance isn't that critical (though this does seem to be slightly
faster).

return (f for f in sorted(self.functions_by_name.itervalues(),
                          key=lambda f: f.offset)
        if f.offset != -1)


Which do you think is more readable?

Dylan

> 
> >
> >      def functionIterateAll(self):
> >          return self.functions_by_name.itervalues()
> > --
> > 2.8.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160401/c6a629d6/attachment.sig>