On Tue, Dec 25, 2012 at 1:33 PM, Glyn Jackson <cfspa...@gmail.com> wrote:
> Hi,
>
> I'm new to Django and just need some guidance.
>
> Below is my attempt at creating a simple app that displays all
> transactions and lists the grand total called 'points' for a particular
> user. Now the following works and does just that, however, I'm not 100% if
> this is the 'correct' way nor really what a 'manager' is.
>
> Could someone look at the following code and advice?
>
> Many, many thanks O, and Happy Christmas everyone!
>
>
> models.py
>
> from django.db import models
> from django.contrib.auth.models import User
>
>
>
>
> class TransactionManager(models.Manager):
>
> def Transaction_list(self,thisUser):
> list = Transaction.objects.filter(user=thisUser)
> return list
>
> def points_total(self,thisUser):
> return
> Transaction.objects.filter(user=thisUser).aggregate(total_points=models.Sum('points'))
>
>
>
> class Transaction (models.Model):
>
> statusOptions = (
> (0, 'Pending'),
> (1, 'Added'),
> (2, 'Deducted'),
> (3, 'Processing'),
> )
>
> user = models.ForeignKey(User)
> points = models.IntegerField(verbose_name=("Points"), default=0)
> created = models.DateTimeField(("Created at"), auto_now_add=True)
> updated = models.DateTimeField(verbose_name=("Updated at"),
> auto_now=True)
> status = models.IntegerField(default=0, choices=statusOptions)
>
> objects = TransactionManager()
>
>
> views.py
>
> from django.template import RequestContext
> from django.contrib.auth.decorators import login_required
> from django.shortcuts import render_to_response
> from points.models import Transaction
>
> @login_required
> def history(request):
>
> thisPoints = Transaction.objects.Transaction_list(request.user)
> totalPoints = Transaction.objects.points_total(request.user)
> context = {'points':thisPoints,'totalPoints':totalPoints}
> return render_to_response('points/history.html', context,
> context_instance=RequestContext(request))
>
> This is certainly reasonable over all.
I would point out, however, that manager instance, "self" in your
Transaction_list and points_total methods, *is* Transaction.object, so
(assuming that you keep them as manager methods), you could just use
"self.filter...". Further, in the interests of not having duplicate code
to maintain, you could implement points_total using
"self.Transaction_list(thisUser).aggregate...". A style point, while it is
customary to use an initial upper case letter on model and manager (and
other class) names, method names, especially those using underscores as
word separators, are usually all lower case. But, too, since it is part of
a TransactionManager, it is a bit redundant to name the method
Transaction_list. You could call it just "list", but perhaps "for_user" is
clearer, allowing you to say (in the view)
"Transaction.objects.for_user(request.user)", which is nearly the simple
English for what you are doing.
It is possible, but not conventional, to create these methods as
classmethods on the Transaction class, rather than requiring the
implementation of a custom manager class, e.g.:
@classmethod
def for_user(cls, thisUser):
return cls.objects.filter(user=thisUser)
You would use this in the view like so:
thisPoints = Transaction.for_user(request.user)
There is a mild preference for the manager approach, since it doesn't
pollute the model name space. A model already has an "objects" attribute,
so accessing the manager doesn't require a new attribute. Using the
classmethod means that there is one more attribute on a model instance,
which should largely be about the instance (table row), rather than about
accessing instances. Not, however, a big deal.
But both of these methods are so simple that hand coding them in the view
has appeal. It makes the code local, rather than forcing the reader to
find the manager code to know for sure what's happening. If the function
were more complex, or if it is used in more than one place, then the
separate manager methods are to be preferred.
And note that you can use a queryset multiple times without having to
recreate it.
@login_required
def history(request):
thisPoints = Transaction.objects.filter(user=request.user)
totalPoints =
thisPoints.aggregate(total_points=models.Sum('points'))
...
The use of "thisPoints" in calculating "totalPoints" doesn't change
"thisPoints". You can still enumerate it in the template, and get the same
result.
Bill
--
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.
