HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythondjangoMinor

School applicant form

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formapplicantschool

Problem

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(

Solution

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.

Code Snippets

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)
form = ApplicantForm(request.POST or None)
if request.method == 'POST':
    if 'last_name' in request.POST:
        form = ApplicantForm(request.POST)
else:
    form = ApplicantForm()
if 'last_name' in request.POST:
    form = ApplicantForm(request.POST or None)
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."),
            },
        }

Context

StackExchange Code Review Q#55290, answer score: 6

Revisions (0)

No revisions yet.