wiki:DeveloperGuidelines/CodeConventions

Version 33 (modified by Dominic König, 11 years ago) ( diff )

--

Code Conventions

Table of Contents

  1. Python
    1. Code Style
      1. PEP8 Script
    2. Quotes
    3. Naming conventions
    4. Comments and Docstrings
    5. Internationalisation
    6. Tools
    7. Exceptions
    8. Nulls
  2. JavaScript
  3. HTML

These conventions should be followed in all code.

NOTE: These coding conventions are mandatory for code to be accepted for the Stable series!

Python

Code Style

PEP8 Script

Use static/scripts/tools/pep8.py to check for ​PEP8 compliance.

Execute the following in your eden directory: 

python static/scripts/tools/pep8.py yourfile.py

Quotes

  • for string constants in Python use double quotes " " - UNLESS the string contains "
  • for string constants in JavaScript use single quotes ' ' - UNLESS the string contains a '
  • for Inline-JavaScript or JSON inside Python code use
  • in all other cases of triple-quoted strings (such as docstrings or Inline-XML) use """ """
  • for XML/HTML attributes use double quotes " " (inner quotes must be escaped anyway)

Naming conventions

  • Function names for the global namespace should start with an "s3_" prefix (if they are not module-specific) or with the module prefix plus underscore.
  • Names for Eden-specific methods in subclasses of web2py classes also start with "s3_".
  • should have the suffix "S3"
  • Names for Eden-specific subclasses of web2py classes and all classes which over-ride existing classes start with "S3",
  • All other classes defined in Eden end with "S3" (e.g. AuthS3 vs. S3Resource).
  • CamelCase is for class names only
  • Constant names are all-uppercase and with underscores as word separator
  • everything else (including table names and field names) should be all-lowercase with underscores as word separator
  • Names should be obvious and explicit, but not overly verbose (i.e. as long as they need to be to not make people think). They shouldn't require someone to look in another file or solve a puzzle to figure out what they mean, but shouldn't take too long to write. Avoid inventing new acronyms. e.g. bad: shn_prm_lkp, method_that_allows_us_to_create_a_gis_layer , good: shn_personnel_search, create_gis_layer

Comments and Docstrings

  • All files, classes and functions should have docstrings which allow to auto-generate API documentation using ​epydoc

Suggested style: 

"""
    This function is just an example
    @param: None
"""

Internationalisation

  • All user-visible strings should be Internationalised: T("My string")
    • Remember to str() them before concatenating with strings (such as HTML tags): "<p>" + str(T("My String")) + "</p>"
  • All labels should be Internationalised in controllers/module.py def resource(): table.field.label = T("My Label")
  • DeveloperGuidelinesInternationalisation

Tools

Exceptions

  • Do not use naked excepts. Yes there are lots already in the code base. Do not make the situation worse. They are being gradually fixed.
    • If you do use a naked except, the exception should be re-raised.
    • database errors are the typical tricky case as we don't want to create dependencies on database modules that aren't used. You can catch BaseException and check the exception class name, or you can provide and reuse standard tuples of exception classes in the configuration that map to the same generic database error.
  • try blocks should be minimised, so that there is less chance of catching the same exception type from a place that wasn't expected, (and then handling it incorrectly). To do this, move code before the exception raising line before the try block, and code after the exception raising line into an else block.
  • It's good practice to clean up resources in finally blocks.
  • avoid raising errors inside exception handlers.
  • don't ignore exceptions. At the very least, log them, or raise them in debug mode.

Instead of this: 

try:
    # this could raise an error we aren't prepared for
    file_path = get_file_path()
    f = file(file_path, 'w')
    # this could also raise an exception
    f.write('Please send the helicopter to %s' % disaster_location)
    f.flush()
except:
    pass
    # Argh! by ignoring the exception, now lots of things could be messed
    # up and the rest of the code could break in strange ways.
    # We will have a hard time figuring out the reason as the original 
    # exception is now lost.
    # Worse, maybe the user doesn't notice and people die on a mountain.

Do this: 

# now we see any errors here separately
file_path = get_file_path()
try:
    # the try block is minimised 
    f = file(file_path, 'w')
except IOError, exception:
    # e.g. a permissions violation
    if session.s3.debug:
        # re-raise in development
        raise
    else:
        # logging is good
        logging.error("%s: %s" % (file_path, exception))

        # email reports are great as the developer doesn't have 
        # to log into the machine to read them.
        # this mustn't raise exceptions (tricky)
        send_error_email_to_devs(exception, locals(), globals(), request)

    if not exception_has_been_totally_handled_and_no_further_consequences():
        # let the user know so they can take action if possible.
        raise HTTPError(500, "Cannot queue your message because: %s" % exception)
else:
    f.write('Please send the helicopter to %s' % disaster_location)
    f.flush()
finally:
    # finalise resources
    f.close()

When should you raise Exceptions, anyway?

If it is not possible to continue with the respective processing because there can not be any plausible result, then the necessary consequences highly depend on the cause:

  • You should never raise an exception for Validation Errors - any invalid user input must be caught and properly reported back to the client (simply because users can't do anything about an exception, so it would render the functionality unusable - whilst a proper error message may help the user to correct their input). Failure to catch validation errors is always a bug.
  • Configuration Errors, wherever possible, should at most issue a warning (if the invalid setting is obviously a mistake), and else get ignored and fall back to reasonable defaults. Only if there is no possible fallback action, then Configuration Errors should be treated as Runtime Errors.
  • Runtime Errors may raise exceptions if unrecoverable (i.e. if there's no reasonable handling possible at the respective level). However, if possible and in production mode, they should be caught at a higher level and properly reported back to the client in order to explain the reason and allow correction. Remember logging!
  • Programming Errors (=bugs) must lead to exceptions in order to attract immediate attention of the programmer to the bug and help them debugging. They should never be caught unless you intend to implement a plausible fallback action (and even then a warning may be appropriate in most cases).

Eden is an unsupervised server system, so you must have very good reasons to let it crash.

Remember that any uncaught exception leads to a HTTP 500 status with error ticket - and an HTTP 500 status in Eden is a bug by definition. Users can't do anything about error tickets, so the only legitimate reason to raise an uncaught exception in Eden is that you are 100% sure that the error condition leading to the exception is caused by a bug.

Nulls

Nulls make code much more complicated by introducing special cases that require special null-checking logic in every client, making code harder to understand and maintain. They often and easily cause hard-to-diagnose errors.

  • Avoid returning nulls wherever possible. Unfortunately the design of python and web2py make this very difficult. E.g. if creating a data structure from many objects, instead of returning nulls from each object and requiring special checking before adding to the structure, pass the data structure to the objects and let them fill in as necessary.
  • Check for nulls and handle them properly unless you are sure you aren't going to get them.
  • Unit tests should check that nulls aren't returned.
  • if you must return a None at the end of the function, do so explicitly, to make the intention clear, rather than falling off the end.
  • Database fields should generally be non-nullable. If there is the chance of unknown data in certain fields, it indicates that the table may be too big, and those fields may need splitting out into separate tables. It may seem reasonable to allow unknown data, but it places the burden of null-checking on every client of the table. If any client doesn't don't do it (the default), then there is a risk of undefined behaviour. By making them non-nullable, we get a level of code validation from the database layer.

JavaScript

HTML

ids & classes should be named ina way that they don't conflict & should be semantic (what they are for) rather than presentational:

DeveloperGuidelines

Note: See TracWiki for help on using the wiki.