On Nov 14, 11:55 pm, Steven D'Aprano <st...@remove-this-
cybersource.com.au> wrote:
> On Fri, 13 Nov 2009 14:10:10 -0800, scoopseven wrote:
> > I actually had a queryset that was dynamically generated, so I ended up
> > having to use the eval function, like this...
>
> > d = {}
> > for thing in things:
> > query_name = 'thing_' + str(thing.id)
> > query_string = 'Thing.objects.filter(type=' + str(thing.id) +
> > ').order_by(\'-date\')[:3]'
> > executable_string = query_name + ' = Thing.objects.filter
> > (type=' + str(thing.id) + ').order_by(\'-date\')[:3]'
> > exec(executable_string)
> > d[query_name] = eval(query_string)
>
> What an unmaintainable mess.
>
> If I've understood it, you can make it less crap by (1) getting rid of
> the unnecessary escaped quotes, (2) avoiding generating the same strings
> multiple times, and (3) avoiding string concatenation.
>
> d = {}
> for thing in things:
> expr = "Thing.objects.filter(type=%s).order_by('-date')[:3]"
> expr = rhs % thing.id
> name = "thing_%s" % thing.id
> exec("%s = %s" % (name, expr))
> d[name] = eval(expr)
>
> What else can we do to fix it? Firstly, why are you creating local
> variables "thing_XXX" (where XXX is the thing ID) *and* dictionary keys
> of exactly the same name? I'm sure you don't need the local variables.
> (If you think you do, you almost certainly don't.) That gets rid of the
> exec.
>
> Next, let's get rid of the eval:
>
> d = {}
> for thing in things:
> x = thing.id
> name = "thing_%s" % x
> d[name] = Thing.objects.filter(type=x).order_by('-date')[:3]
>
> About half the size, ten times the speed, and 1000 times the readability.
>
> Unless I've missed something, you don't need either exec or eval.
>
> --
> Steven
Steven,
This worked like a charm! Thank you for your insight, it's greatly
appreciated.
Mark
--
http://mail.python.org/mailman/listinfo/python-list
