On Mon, Jul 25, 2011 at 4:57 AM, nixlists <nixmli...@gmail.com> wrote:
> Hi. I am a newbie  :)
>
> Is there a way to rewrite the dictionary code above to make it more
> readable, and/or rewrite everything in a more efficient manner in
> general? It still takes a bit of time to run (but much faster than
> doing queries inside the loop).
>
> If I am doing anything glaringly wrong, or if you have any other
> suggestions please let me know. I really appreciate your help.
>
> Thanks.
>

I've rearranged your mail to make it a bit easier to answer. You are
on the right track with your approach here, but you are doing too much
in loops, which is why you feel it is slow.


>    c = Contract.objects.get(id = contract_id)
>
>    p_per_c=c.products.all()
>
>    c_dict = dict((product.id, product.contractproduct_set.get(
>        contract=contract_id,ndc=product.id).rebate_pct)
>        for product in p_per_c)
>
>    p_dict = dict((product.id, product.contractproduct_set.get(
>        contract=contract_id, ndc=product.id).wac.wac)
>        for product in p_per_c)


So, this is the critical section of your code. You are trying to build
dictionaries of data that you can then use later on, to avoid hitting
the database repeatedly. However, the queries you use here are a bit
naive, and mean you have to do many similar queries to build up your
data sets. This is where the code can be improved.

c_dict seems to contain the product id => rebate_pct from the
appropriate ContractProduct for each Product associated with this
Contract, but it must perform one query to get the list of products,
and one query per product to get the rebate_pct. This could be
simplified - there are many ways to approach this, but I like to start
from the thing that I want - the ContractProduct:

  ContractProduct.objects.filter(contract=c)

should produce all the items we are interested in for this contract.
The next performance tip is to not fetch and create django model
instances when all you want is the data - just ask for it using
values() or values_list(), hence:

  contract_products = ContractProduct.objects.filter(contract=c)
  c_dict = dict(contract_products.values_list('product', 'rebate_pct'))

p_dict, unless I've misread, is just slightly different data from the
same data set, and hence applying the same approach:

  p_dict = dict(contract_products.values_list('product', 'wac__wac'))

This should cut down the number of queries in the view significantly.

Cheers

Tom

-- 
You received this message because you are subscribed to the Google Groups 
"Django users" group.
To post to this group, send email to django-users@googlegroups.com.
To unsubscribe from this group, send email to 
django-users+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-users?hl=en.

Reply via email to