QuerySets in Dictionaries

[Steven D'Aprano]

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