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

Django view and template for a virtual zoo

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

Problem

I have a Django view and template that work together, but I think I probably did things in a crappy way. I basically have many dynamic values that I want to display on the page, and in two different ways. When the user first comes to the page, these values get returned in the normal context and are stored in javascript variables. When the user clicks a button, an ajax request is fired off, and returns json, which then updates the js variables. I think I could have done it better. Beyond that, I am a Django/python beginner and appreciate any tips that would help me improve.

index.html

```
{% extends "base.html" %}
{% load i18n %}

{% block head %}
{% load static %}
{% endblock %}

{% block sub-nav %}
{% if user.is_authenticated %}
{% trans 'My Contributions' %}
{% trans 'All Contributions' %}
{% endif %}
{% endblock %}

{% block content %}



var current = 0;
var stats = {

'meals':"{{calculations.meals}}",
'animals_saved': "{{calculations.animals_per_meal}}",
'co2_per_meal': "{{calculations.co2_per_meal}}",
'water_per_meal': "{{calculations.water_per_meal}}",
'grain_per_meal':"{{calculations.grain_per_meal}}"
};
function decrement(){
if(current == 0)
current = 4
else
current--;
}

function increment(){
if(current == 4)
current = 0;
else
current++;
}
function changeMainContent(tab, stats){
text = "";

if(tab == 0)
text += stats.meals
else if(tab == 1)
text += stats.animals_saved
else if(tab == 2)
text += stats.co2_per_meal
else if(tab == 3)
text += stats.water_per_meal
else if(tab == 4)
text += stats.grain_per_meal

text += "";
$('#main-number').html(text);
}

$(document).ready(function() {
$('#mealClick').click(function() {
var meals = $('#mealsToday option:selected').text();
var auto_update = $('#vegetarian_cb').prop('checked');

Solution

Naming

In update_count, the variable v is very poorly named.
It seems that meals_today would reflect its purpose,
and that would be a much better name.

Simplifications

In update_count, you check if v is None a few times.
You could simplify the code by initializing its value to 0 instead of None, so:

meals_today = 0


Then, instead of this:

total_meals = mealCount
    if(v is not None):
        total_meals += v


You can write simpler as:

total_meals = mealCount + meals_today


And instead of this:

if v is not None:
    v += mealCount
else:
    v = mealCount


You can write simpler as:

meals_today += mealCount


Naming 2

Some variables are named camelCase and others snake_case.
The convention in Python is to use snake_case,
consistently, everywhere.

It's ok to use a different convention for variables in JavaScript and the cookies,
that's up to you.
For Python there is a standard (snake_case), and it's good to follow that.

Indenting

The indenting is off at many places in the JavaScript code, for example:

function decrement(){
        if(current == 0)
                current = 4 
    else
        current--;
}


The clean way to write:

function decrement(){
    if (current == 0) {
        current = 4; 
    } else {
        current--;
    }
}


Notice that I also added braces { ... }, a recommended practice,
and a statement terminating ; was missing too.

Passing data to the template

You seem concerned about the way you pass data to the template. And then how the context is used to store these values in JavaScript. It doesn't seem so bad to me. I don't really have groundbreaking ideas to improve this part, it seems quite done the way it is now.

The only thing is, I would drop the term "context" from the function name build_calculations_context. Because the dictionary of calculations that this creates is only one part of the context (in Django's terms) that you pass to the template, which can be confusing, misleading readers that this dictionary is the whole context. For the same reason, also avoid assigning this dictionary to variables named context.

Code Snippets

meals_today = 0
total_meals = mealCount
    if(v is not None):
        total_meals += v
total_meals = mealCount + meals_today
if v is not None:
    v += mealCount
else:
    v = mealCount
meals_today += mealCount

Context

StackExchange Code Review Q#96408, answer score: 4

Revisions (0)

No revisions yet.