|Version 20 (modified by 10 years ago) ( diff ),|
Table of Contents
These conventions should be followed in all code.
NOTE: These coding conventions are mandatory for code to be accepted for the Stable series!
- Limit line length to 80 characters
- Use " " for strings, UNLESS the string contains a ", in which case use '
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
- 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:
Comments and Docstrings
- All files, classes and functions should have docstrings which allow to auto-generate API documentation using epydoc
""" This function is just an example @param: None """
- All user-visible strings should be Internationalised:
- Remember to
str()them before concatenating with strings (such as HTML tags):
"<p>" + str(T("My String")) + "</p>"
- Remember to
- All labels should be Internationalised in
controllers/module.py def resource():
table.field.label = T("My Label")
- Some automated bug analysis / code quality checking tools -
- 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.
# 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()