Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have view and it works correct, but very slow

class Reading(models.Model):
    meter = models.ForeignKey(Meter, verbose_name=_('meter'))
    reading = models.FloatField(verbose_name=_('reading'))
    code = models.ForeignKey(ReadingCode, verbose_name=_('code'))
    date = models.DateTimeField(verbose_name=_('date'))

    class Meta:
        get_latest_by = 'date'
        ordering = ['-date', ]

    def __unicode__(self):
        return u'%s' % (self.date,)

    @property
    def consumption(self):
        try:
            end = self.get_next_by_date(code=self.code, meter=self.meter)
            return (end.reading - self.reading) / (end.date - self.date).days
        except:
            return 0.0

    @property
    def middle_consumption(self):
        data = []
        current_year = self.date.year
        for year in range(current_year - 3, current_year):
            date = datetime.date(year, self.date.month, self.date.day)
            try:
                data.append(Reading.objects.get(
                    date = date,
                    meter = self.meter,
                    code = self.code
                ).consumption)
            except:
                data.append(0.0)
            for i in data:
                if not i:
                    data.pop(0)
        return sum(data) / len(data)

class DataForDayChart(TemplateView):
    def get(self, request, *args, **kwargs):
        output = []
        meter = Meter.objects.get(slug=kwargs['slug'])
        # TODO: Make it faster
        for reading in meter.readings_for_period().order_by('date'):
            output.append({
                "label": reading.date.strftime("%d.%m.%Y"),
                "reading": reading.reading,
                "value": reading.consumption / 1000,
                "middle": reading.middle_consumption / 1000
            })
        return HttpResponse(output, mimetype='application/json')

What should I change to make it faster?

share|improve this question
    
How slow it is? How many records the view returns? Looks like you have a loots of SQL Queries there, try to reduce amount of queries. –  srusskih Dec 24 '12 at 8:18

1 Answer 1

There's nothing intrinsically time-consuming here code-wise - I think the best approach is to make sure that your database tables are set up correctly with the required indices. Perhaps create a view on the db to push some work onto that rather than select in code?

I am a little puzzled however by your middle_consumption function. The inner loop contents do something like:

get a date, x years ago from present day
get a reading on that date, or 0 on failure
add that reading to the results
go through the results
    if the current item is 0, delete the **first** item

This seems wrong.

  1. What happens on February 29th? It would be better to add 365 days if that's appropriate for your application.
  2. Why add a value only to (presumably want to) delete it again? Would something like this be better?

    try:
        current_value = Reading.objects.get(
            date = date_of_reading,
            meter = self.meter,
            code = self.code
        ).consumption
    except Reading.DoesNotExist: # or suitable exception
        current_value = 0
    if current_value > 0:
        anniversary_values.append( current_value )
    
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.