Skip to content

Coding style

danny edited this page Mar 1, 2024 · 26 revisions

↑ Parent: More

← Previous: List of common MagiCircles prefixes and suffixes

  1. All languages
    1. Naming
    2. Spacing
    3. Commenting
    4. Lingo
  2. Python
    1. Order of imports
    2. Types
    3. Don't concatenate translated strings
    4. Avoid calling functions in templates
  3. Models
    1. Order of content within a model
    2. Optimize queries
    3. Create utility functions in models
    4. Models string representation ("unicode")
  4. Collections
    1. Order of content within a collection
  5. Forms
    1. Order of content within a form

ℹ️ See also Commit your changes

All languages

  • Indent with 4 spaces, no tab.

  • For any user-facing text such as model field names, help text, etc, as well as code comments, use sentence case:

    • Join The Community!Join the community!
    • Favorite CardsFavorite cards
  • Group related logic together and separate with new lines:

    Instead of: ❌

    total_strawberries = 0
    total_bananas = 0
    for strawberry in strawberries:
        total_strawberries += 1
    for banana in bananas:
        total_bananas += 1

    Do: ✅

    total_strawberries = 0
    for strawberry in strawberries:
        total_strawberries += 1
    
    total_bananas = 0
    for banana in bananas:
        total_bananas += 1

Naming

  • For global functions, use camelCase.
  • For global variables, use UPPERCASE_SNAKE_CASE.
  • For classes, use PascalCase.
  • For classes methods and variables, use snake_case.

Variables/functions names should be clear. Anyone reading the code later should know what the variable/function is for. Avoid generic names such as data, var1, tmp, flag, etc.

For clarity as well, avoid abbreviations, single-letter names, and acronyms, in both texts and variables/functions names:

  • card_stats -> card_statistics
  • site_config -> site_configurations
  • param1 -> parameter1
  • dsp -> database_server_password
  • k, v -> key, value

Spacing

  • Variable assignment: a = 5
  • Function parameters: a(b, c, d)
  • Named parameters: a(b=c, d=e)
  • Dictionary / objects:
    a = {
        'b': c,
        'd': 'e',
    }
    Note the trailing comma on the last element.
  • One-line dictionary / objects / lists:
    a = { 'b': c, 'd': 'e' }
    a = [ a, b, c ]
    Note the spaces surrounding elements.

Commenting

  • Python:

    ############################################################
    # Name of the section
    
    some_code()
    
    # Smaller section
    
    some_code() # a comment
  • Javascript:

    // *****************************************
    // Name of the section
    
    some_code()
    
    // Smaller section
    
    some_code() // a comment
  • LESS/CSS:

    /******************************************/
    /* Name of the section */
    
    some_code()
    
    /* Smaller section */
    
    some_code() /* a comment */
    

Lingo

  • _('Name') should only refer to the name of a person. For anything else, use _('Title').
  • We generally use _('Origin') instead of _('Source')
  • We use the term _('Version') to refer to game versions, instead of _('Server').
  • Using columns to separate messages is not a standard practice in some languages. For this reason, to keep our formatted messages international-friendly, instead of doing: '{foo}: {bar}'.format(...), we do: {foo} - {bar}.format(...)

Python

Order of imports

  1. Standard libraries
  2. Installed libraries (installed by requirements.txt)
  3. Django imports
  4. MagiCircles imports
  5. Project imports

Within each section, sort alphabetically.

Types

Prefer using list to tuple when listing stuff:

  • Don't: ❌
    translated_fields = ('name', 'description')
  • Do: ✅
    translated_fields = [ 'name', 'description' ]

When creating a dict, if you need to include elements using a forloop:

  • Don't: ❌
    fields_icon = {
        'event': 'event',
    }
    fields_icons.update({
        image_field_name: 'pictures'
        for image_field_name in models.Card.IMAGES
    })
  • Do: ✅
    from magi.utils import updatedDict
    fields_icons = updatedDict({
        'event': 'event',
    }, {
        image_field_name: 'pictures'
        for image_field_name in models.Card.IMAGES
    })
  • If the dict needs to be copied (common in inheritance), do: ✅
    from magi.utils import mergeDicts
    fields_icons = mergeDicts(ParentCollection.fields_icons, {
        'event': 'event',
    })

Don't concatenate translated strings

In some languages, the order of the words in a sentence might not be what you expect. If you force the order of words with concatenation, you make the translators' job very hard, if not impossible.

Example in python: ❌

context['greetings'] = _('Hello') + request.user.username + '!'

should be: ✅

context['greetings'] = _('Hello {name}!').format(name=request.user.username)
# or, in a global context:
__(_('Hello {name}!'), name=_('Guest'))

Example in template: ❌

{% trans 'Hello' %} {{ user.username }}!

should be: ✅❋

{% load tools %}
{% trans_format string='Hello {name}!' name=user.username %}

⚠️ Note that in general, it is recommended to do that transformation in python and not in the template, since doing it in template requires the inclusion of the tools module, which adds a significant cost.

Avoid calling functions in templates

While Django provides templatetags, it is recommended to avoid having too many load inside your template, especially when it's likely to be included multiple times (in a forloop for example), because they slow down the generation of the HTML page.

Instead, try to provide as much as possible of the logic behind your template in your python code, and add variables inside your context.

Example:

  • Instead of: ❌
    {% load mytags %}
    <div>{% get_percent a b %}</div>
  • Do: ✅
    context['percent'] = get_percent(a, b)
    <div>{{ percent }}</div>

Models

Order of content within a model

For each model in ${PROJECT}/models.py:

  • Title before the class

  • collection_name as the first line

  • IS_PERSON = True (if applies)

  • TRANSLATED_FIELDS = [ ... ] (if applies)

  • owner = ...

  • followed by a new line

  • Fields (can be separated in sections with titles when relevant)

  • Fields order should match order in which they'll be displayed in the item view

  • Fields that come with options (example: i_rarity comes with option RARITY_CHOICES) should have their options right before or right after the field, with no new line between.

  • If you provide utility function related to a specific field, it should be right after the field declaration. Unless it's related to how it's displayed, in that case, it should be under Views utils section.

  • Internal fields (not visible to users) should show up at the end of all other fields, with a title "Internals" -After the fields, the following sections, with their title:

${PROJECT}/models.py:

############################################################
# Idol

class Idol(MagiModel):
    collection_name = 'idol'
    IS_PERSON = True
    TRANSLATED_FIELDS = [ 'name' ]
    owner = models.ForeignKey(User, related_name='added_idols')

    ############################################################
    # Main details

    name = models.CharField(_('Name'), max_length=100, null=True)
    NAMES_CHOICES = NON_LATIN_LANGUAGES
    d_names = models.TextField(_('Name'), null=True)

    ...

    ############################################################
    # Statistics (example of other section to separate fields)

    ...

    ############################################################
    # Internals

    game_id = models.PositiveIntegerField(null=True)

    ############################################################
    # Cache

    _cache_total_fans = models.PositiveIntegerField(null=True)
    ...

    ############################################################
    # Reverse relations

    REVERSE_RELATED = [ ... ]

    ############################################################
    # Image settings

    tinypng_settings = {
	'front': {
            'resize': 'scale',
	    'height': 320,
	}
    }

    ############################################################
    # Views utils

    display_price = property(lambda _s: u'${}'.format(_s.price))

    ############################################################
    # Unicode

    def __unicode__(self):
	return u'{} - {}'.format(self.display_name or _('Costume'), self.idol.t_name)

    ############################################################
    # Meta

    class Meta:
        ordering = [ 'name' ]

Optimize queries

Add db indexes

Checklist:

  • Default ordering for each collection
  • fields_prefetched_together, fields_preselected, fields_prefetched
  • Presets

Don't add indexes to item and owner under collectibles.

N+1

A common problem in programming is the N+1 problem.

Let's say you retrieve the list of cards:

cards = models.Card.objects.all()

For each card you want to display the id, rarity and idol id:

for card in cards:
    print card.id
    print card.rarity
    print card.idol_id

In this case, one SQL query will be performed:

SELECT * FROM cards

But let's say you want to display the name of the idol instead of the id:

for card in cards:
    print card.id
    print card.rarity
    print card.idol.name

Code looks pretty short and innocent, right?

Well, in reality, it will only see that you need to retrieve the idol details after it did the first query, so in the end, you will have as many queries as you have cards (!!!):

SELECT * FROM cards
SELECT * FROM idols WHERE id=12
SELECT * FROM idols WHERE id=4
SELECT * FROM idols WHERE id=1
SELECT * FROM idols WHERE id=8
SELECT * FROM idols WHERE id=12
SELECT * FROM idols WHERE id=4
SELECT * FROM idols WHERE id=1
...

This can have a huge impact on loading time and server load.

To detect it, log queries and errors (see Local setup, "Log queries or errors" section) and browse your local website. Check the queries printed to see if there is an N+1 issue.

To fix N+1 issues, we have 2 solutions:

  • 1st solution: Make a JOIN request to retrieve the idol details as well:

    cards = models.Card.objects.all().select_related('idol')

    The query will then look like this:

    SELECT card.*, idol.* FROM card JOIN idol WHERE card.idol_id=idol.id

    The query will then be significantly slower than the original query, but still waaaaaay faster than the addition of all the queries.

    This is recommended for most pages, such as list of idols or events.

  • 2nd solution: Add a cache within the model

    This is recommended for pages that have a very high traffic, such as cards page. It's recommended to use the 1st solution until we know for sure the page gets significantly more traffic.

    See Use an internal cache for foreign keys in models.

Create utility functions in models

It's very likely that you'll need to do something to the fields of your models, and you won't be using most of them as they are stored in the database. In that case, it's recommended to add properties to your model classes.

Example:

  • Instead of:
    today = datetime.date.today()
    idols = models.Idol.objects.all()
    for idol in idols:
        idol.age = today.year - idol.birthdate.year - ((today.month, today.day) < (idol.birthdate.month, idol.birthdate.day))
  • Do: ${PROJECT}/models.py:
    class Idol(MagiModel):
        ...
        birthdate = models.DateField(_('Birthdate'))
    
        @property
        def age(self):
            return today.year - self.birthdate.year - ((today.month, today.day) < (self.birthdate.month, self.birthdate.day))
    {% for idol in idols %}
    <div>{{ idol.age }}</div>
    {% endfor %}

Models string representation ("unicode")

__unicode__ is a method that's used to retrieve a string representation of any object, including models.

In MagiCircles, this method is called all the time, all around the websites.

For SEO purposes, and because we care about our international fans, it's important that you try to localize the name of your items whenever you can and whenever it's relevant. See Translate fields values in multiple languages.

${PROJECT}/models.py:

class Card(MagiModel):
    name = models.CharField(_('Title'), max_length=100)
    NAMES_CHOICES = ALL_ALT_LANGUAGES
    d_names = models.TextField(_('Title'), null=True)

When you have a translated name field in your model, __unicode__ will call t_name, and you don't need to override it. This works in most cases.

But there are cases where just a name won't do, and you may want to have a more complex __unicode__ method. In this example, we want to display the rarity of a card before its name:

${PROJECT}/models.py:

class Card(MagiModel):
    i_rarity = ...

    def __unicode__(self):
      return u'[{rarity}] {name}'.format(rarity=self.t_rarity, name=self.t_name)

In some cases, we may even want to use some details within a foreign key. In this example, the name of the idol linked to a card:

${PROJECT}/models.py:

class Card(MagiModel):
    idol = models.ForeignKey(...)

    def __unicode__(self):
      return u'{idol} - {name}'.format(idol=self.idol.t_name, name=self.t_name)

In that case, it's extremely important to ensure that everywhere this model can be used, the idol instance is always preselected. To achieve that, make sure the queryset of its associated collection has the right select_related:

${PROJECT}/magicollections.py:

class CardCollection(MainItemCollection):
    queryset = models.Card.objects.select_related('idol')

It's still recommended to test out extensively that there aren't any case where the query doesn't preselect it. To do so, log queries and errors (see Local setup, "Log queries or errors" section) and browse your local website, then check the queries. There should be a JOIN in your card query, not a separate SELECT query to fetch the idol.

Another solution if you want to use a detail from a foreign key within your __unicode__ method is to use an internal cache (see ⎡Use an internal cache for foreign keys in models⎦).

In that case, make sure you only do it when the object has actually been created, like so:

${PROJECT}/models.py:

def __unicode__(self):
    if self.pk:
        return u'{name} - {rarity}'.format(name=self.cached_idol.t_name, rarity=self.t_rarity)
    return u'{rarity}'.format(rarity=self.t_rarity)

Collections

Order of content within a collection

  • queryset should always be first.
  • Followed by name, plural_name, title and plural_title.
  • Then any other collection setting that just sets a variable
  • Then any method
  • Views should be in this order:
    • List view
    • Item view
    • Add view
    • Edit view

If a setting is not changed from its default value, or if a method doesn't do anything, it shouldn't be specified. For example, idol is already the default name of this collection, so it doesn't need to be specified.

Do: ❌ ${PROJECT}/magicollections.py:

class IdolCollection(...):
    name = 'idol'
    ...

Do: ✅

class IdolCollection(...):
    ...

Same for views, if none of the views default settings are overrided, the view class shouldn't show up at all:

Do: ❌ ${PROJECT}/magicollections.py:

class IdolCollection(...):
    ...
    class AddView(...):
        pass

Do: ✅

class IdolCollection(...):
    ...

Forms

Order of content within a form

  • Any form setting that's not a form field, such as:
    • search_fields
    • search_fields_exact
    • search_fields_labels
    • ordering_fields
    • merge_fields
    • presets
    • show_more
    • modal_cuteform_separators
    • on_change_value_show
    • on_change_value_trigger
    • cuteform
  • Form fields
    • Note that most of the time, model fields don't need to be specified. Just having them in Meta fields should trigger their default behavior, widget, etc.
    • In MagiFilterForm, if you need to specify a MagiFilter in {}_filter, it should be right under the field.
  • __init__, if needed to be overrided
    • For example, to update some details within the form fields
  • clean_{} for fields with custom clean logic (see Django doc)
  • clean, global clean (see Django doc)
  • save
  • class Meta to specify model and fields, and some settings, such as:
    • hidden_fields
    • hidden_foreign_keys
    • save_owner_on_creation

I. Introduction

II. Tutorials

  1. Collections
    1. MagiModel
    2. MagiCollection
    3. MagiForm
    4. MagiFiltersForm
    5. MagiFields
  2. Single pages
  3. Configuring the navbar

III. References

IV. Utils

V. Advanced tutorials

VI. More

Clone this wiki locally