wiki:DeveloperGuidelines/CodeConventions

Version 20 (modified by Mike A, 10 years ago) ( diff )

added notes on naming conventions

Code Conventions

These conventions should be followed in all code.

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

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

Naming conventions

  • All functions outside of classes should have the prefix shn_<Model Name>_
  • All classes which over-ride existing classed should have the suffix "S3"
  • 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 deployment_settings.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()


DeveloperGuidelines

Note: See TracWiki for help on using the wiki.