7
\$\begingroup\$

I have created, in Django 1.6, a site for a school, where an applicant can get registered. It's a form that requests some information from the registrant and creates a random code for each applicant. That code is emailed to the provided address, and it is requested to be input in another form that appears right after submitting the first one.

I am completely sure that it can be written better.

####  models.py #####

#coding=utf-8

from django.db import models
from .utils import LEVELS, GENDER,SCHOOL_YEAR
from django_countries.fields import CountryField
from django.utils import timezone
from django.forms import Form, ModelForm
from datetime import datetime
import string
import random

class Applicant(models.Model):
    first_name=models.CharField('Nombres',max_length=30)
    last_name=models.CharField('Apellidos',max_length=30)
    gender=models.CharField('Género',max_length=2,choices=GENDER)
    grade_applied_for=models.CharField('Grado al que aplica',max_length=6,choices=LEVELS)
    birth_date=models.DateField('Fecha de nacimiento')
    birth_place=CountryField('País de nacimiento')
    residence_place=CountryField('País de residencia')
    address=models.CharField('Dirección',max_length=80)
    rep_name=models.CharField('Nombres del representante',max_length=40)
    rep_email=models.EmailField('Correo electrónico del representante')
    rep_landline=models.CharField('Número de teléfono convencional',max_length=20)
    rep_mobile=models.CharField('Número de teléfono móvil',max_length=10)
    prev_school_name=models.CharField('Insitución Educativa previa',max_length=20,blank=True)
    prev_school_place=models.CharField('Lugar de la insitución',max_length=20,blank=True)
    last_approved_year=models.CharField('Ultimo año lectivo aprobado',max_length=4,choices=SCHOOL_YEAR,blank=True)
    invoice_name=models.CharField('Razón Social',max_length=20)
    invoice_id_number=models.CharField('Nómero de cédula o RUC',max_length=10)
    invoice_address=models.CharField('Dirección',max_length=40)
    invoice_phone=models.CharField('Teléfono',max_length=15)
    pub_date = models.DateTimeField('Fecha de aplicacion',default=datetime.now,unique=True)

    confirmation_code = models.CharField('código de confirmación',max_length=6)
    confirmation_status=models.BooleanField(default=False)

    def __unicode__(self):
        return self.first_name+" "+self.last_name


#### views.py #####

from django.shortcuts import render, get_object_or_404, render_to_response
from applications.models import Applicant
from applications.forms import ApplicantForm, ConfirmationCode
from django.http import HttpResponse
from django.template import RequestContext, loader
from django.core.mail import send_mail
import string
import random

def random_confirmation(size=6,chars=string.digits):
    return ''.join(random.choice(chars) for _ in range(size))


def index(request):

    latest_application = Applicant.objects.all().order_by('-pub_date')
    context = {'latest_application':latest_application}
    return render(request,'applications/index.html',context)

def add_applicant(request):
    context = RequestContext(request)
    if request.method == 'POST':
        if 'last_name' in request.POST:
            form = ApplicantForm(request.POST)
            if form.is_valid():
                applicant = form.save(commit=False)
                applicant.confirmation_code = random_confirmation()
                applicant.save()
                send_mail('Confirmacion de aplicacion','Su codigo de confirmacion es %s' %applicant.confirmation_code,'[email protected]',[applicant.rep_email],fail_silently = True)
                form2 = ConfirmationCode(initial={'confirmation_id':applicant.pk})
                return render_to_response('applications/confirmation.html',{'applicant':applicant,'pub_date':applicant.pub_date,'confirmation_code':applicant.confirmation_code,'form':form2},context)
            else:
                print form.errors
                return render_to_response('applications/add_applicant.html',{'form':form},context)

        elif not 'last_name' in request.POST:
            form = ApplicantForm(request.POST)
            code  = request.POST['confirmation_code']
            confirmation_id = request.POST['confirmation_id']
            applicant = Applicant.objects.get(pk=confirmation_id)
            if code == applicant.confirmation_code:          

            Applicant.objects.filter(pk=confirmation_id).update(confirmation_status=True)
                    return render_to_response('applications/succedded.html'{'applicant':applicant},context)
                else:
                error = 'Codigo incorrecto'
                form2 = ConfirmationCode(initial={'confirmation_id':applicant.pk})
                return render_to_response('applications/confirmation.html',{'applicant':applicant,'pub_date':applicant.pub_date,'confirmation_code':applicant.confirmation_code,'form':form2,'error':error},context)

    else:
        form = ApplicantForm()
        return render_to_response('applications/add_applicant.html',{'form':form},context)

#### forms.py ####
#coding=utf-8

from django.db import models
from .utils import LEVELS, GENDER,SCHOOL_YEAR
from django_countries.fields import CountryField
from django.utils import timezone
from django.forms import CharField, Form, ModelForm, HiddenInput
from applications.models import Applicant
from datetime import datetime
import string
import random


def random_confirmation(size=6, chars=string.digits):
    return ''.join(random.choice(chars) for _ in range(size))

class ApplicantForm(ModelForm):
    class Meta:
        model = Applicant
        fields  = ['first_name','last_name','gender','grade_applied_for','birth_date','birth_place','residence_place','address','rep_name','rep_email','rep_landline','rep_mobile','prev_school_name','prev_school_place','last_approved_year','invoice_name','invoice_id_number','invoice_address','invoice_phone']

class ConfirmationCode(Form):
    confirmation_id = CharField(required=True,max_length=10,widget=HiddenInput())
    confirmation_code = CharField(max_length=6)

#### urls.py ####
from django.conf.urls import patterns, include, url

from applications import views

urlpatterns = patterns('',
    url(r'^$',views.index, name='index'),
#    url(r'^(?P<applicant_id>\d+)/$',views.detail,name='detail')
    url(r'^add_applicant/$',views.add_applicant,name='add_applicant'),
)
\$\endgroup\$
1
  • \$\begingroup\$ One of the solutions I applied that botters me the most is the fact that I had to use to diferent forms in the same view function "add_applicant", and to identify those, I determined the presence or absence of a field that is present in the first form.POST but is not in the second one. First I tried to make to different functions, but I could figure out how to take some db info (the applicant pk) from one vew to the other. \$\endgroup\$ Commented Jun 26, 2014 at 13:21

1 Answer 1

6
\$\begingroup\$

One thing that concerns me about this approach is that Applicant is huge. I'd consider splitting this into a few different models. At the very least I'd imagine that I'd want to split out the details for a person and contact information:

class Person(models.Model):
    """Information for a person"""
    first_name=models.CharField('Nombres',max_length=30)
    last_name=models.CharField('Apellidos',max_length=30)
    gender=models.CharField('Género',max_length=2,choices=GENDER)
    birth_date=models.DateField('Fecha de nacimiento')
    birth_place=CountryField('País de nacimiento')
    residence_place=CountryField('País de residencia')

class ContactInfo(models.Model):
    """Contact information for a Person"""
    address=models.CharField('Dirección', max_length=80)
    rep_name=models.CharField('Nombres del representante', max_length=40)
    rep_email=models.EmailField('Correo electrónico del representante')
    rep_landline=models.CharField('Número de teléfono convencional', max_length=20)
    rep_mobile=models.CharField('Número de teléfono móvil', max_length=10)

class Applicant(models.Model):
    """Information about an application"""
    person = models.ForeignKey(Person)
    contact_info = models.ForeignKey(ContactInfo)
    prev_school_name=models.CharField('Insitución Educativa previa', max_length=20, blank=True)
    prev_school_place=models.CharField('Lugar de la insitución', max_length=20, blank=True)
    last_approved_year=models.CharField('Ultimo año lectivo aprobado', max_length=4, choices=SCHOOL_YEAR, blank=True)
    invoice_name=models.CharField('Razón Social', max_length=20)
    invoice_id_number=models.CharField('Nómero de cédula o RUC',max_length=10)
    invoice_address=models.CharField('Dirección', max_length=40)
    invoice_phone=models.CharField('Teléfono', max_length=15)
    pub_date = models.DateTimeField('Fecha de aplicacion', default=datetime.now, unique=True)

I also added some docstrings here as I think that's generally speaking a very good idea as it can let people get familiar with the structure of your code faster and can allow you to use documentation generation tools.

Another thing you can do to reduce code duplication is to instantiate forms like this:

form = ApplicantForm(request.POST or None)

This would allow you to remove a conditional in your view code, so instead of:

if request.method == 'POST':
    if 'last_name' in request.POST:
        form = ApplicantForm(request.POST)
else:
    form = ApplicantForm()

you can just do:

if 'last_name' in request.POST:
    form = ApplicantForm(request.POST or None)

Reducing the number of branches in the code is definitely a good thing when it comes code maintenance and keeping things manageable.

Also with your ModelForms you have a chance to add things like help text and error messages:

class ApplicantForm(ModelForm):
    class Meta:
        model = Applicant
        fields  = ['first_name','last_name','gender','grade_applied_for','birth_date']
        help_texts = {
            'birth_date': _('The day someone was born.'),
        }
        error_messages = {
            'name': {
                'max_length': _("This applicant's name is too long."),
            },
        }

One other thing is that sometimes you let lines of code get very long. For example:

fields  = ['first_name','last_name','gender','grade_applied_for']

Could be much more clearly written as:

fields  = [ 
    'first_name',
    'last_name',
    'gender',
    'grade_applied_for',
]

Tools such as pylint would complain about the line length there because after a certain line size the readability of the code is negatively impacted.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.