On 19 January 2011 18:06, John <jspay...@verizon.net> wrote:
> def reports_list(request, period_id)
>   # some other irrelevant code
>   try:
>      reportheader=ReportHeader.objects.get(pk=1
>   except Exception, e:
>      pass
>   return render_to_response("reports_list.html, locals(),
> context_instance=RequestContext(request))

1) You don't want to modify __init__() of your model like this:

  def __init__(self, request, *args, **kwargs)
     self.provision=request.provision
     self.period=self.provision.get_current_period()

If the new constructor requires an additional "request", how does the
ORM create instances of it ? It doesn't know anything about request.
You can make a helper function. Or if you really want to be a part of
the class, make an alternative constructor:

   @classmethod
   def from_provision(cls, provision):
        instance = cls()
        instance.provision = provision
        instance.perio = provision.get_current_period()

    # in your view code::

    header = ReportHeader.from_provision(requrest.provision)

2) The moment you wrote "locals()" all the code in your view becomes
relevant, because you're passing all the local variables to your
template. This seems very popular amongst some groups, but I
personally consider this not only bad style, but error-prone and
possibly dangerous.

3) Catching exceptions like that most likely masks the real problem
(Like the fact, that creating an instance of ReportHeader failed, due
to buggy constructor). After all, you're rendering the template even
if you failed to fetch the data you most likely need to render it.

-- 
Łukasz Rekucki

-- 
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