Table of Contents

• Introduction
• Recommendation
  • Call Parent Method For Zope Hook Method Override In Python
  • Code Should Work If All Else Fails
  • Comments Should Be Used Wisely
  • Prevent Acquisition From Acquiring Restricted Properties In Python
  • Python Class Method Should Have Security Declaration
  • Use Of JavaScript Globals Is Discouraged
• Rule
  • A Non Anonymous JavaScript Method Must Be Declared Before Use
  • Abbreviations Are Not Allowed
  • Code Must Be Validated With JsLint
  • Class And Id Attributes Are Lowercase With Underscore Separator
  • JavaScript Closing Bracket Is At Indent Of Function Call
  • JavaScript Constructor Starts With Captial Letter
  • JavaScript Opening Brace Is On Line Of Current Statement
  • Method Name Is Camelcase Starting With Lowercase Letter
  • Never Hardcode If Not Necessary
  • Plural Is Not Allowed
  • Private Method Starts With Underscore
  • Sessions Are Not Allowed
  • Use Methods To Access Properties In ERP5
  • Use Of Id Attribute Is Not Allowed
  • Use Two Space Indentation
• Crime
  • Cookies Are Not Allowed
  • FailUnless Should Not Be Used In Python Unit Tests To Test Identity
  • Never Access state_change Object In ERP5 Workflow Scripts
  • Never Hardcode Interactions
  • Never Hide Exceptions
  • Never Modify 100 Objects In A Transaction
  • Never Name A Base Category The Same As A Property In ERP5
  • Never Override Python Builtin Names
  • Never Store Calculation Result In Python Property
  • Never Use aq_parent To Get Document Parent In Python
  • Never Use ContentValues or ObjectValues On More Than 50 Documents In ZODB
  • Never Use DoActionFor In ERP5
  • Never Use GetPath To Retrieve A Url In ERP5
  • Never Use Python If Obj if Obj Is Not A Simple Type
  • Never Never Use Reindex In ERP5
  • Never Use Temp ZODB Folder Across Multiple Requests
  • Never Write To ZODB If Not Required

Introduction¶

These guidelines have been written to ensure all code written is clean, maintainable and accessible code ensureing that code is resuable and can be understood in the future by anyone having to work on it.

While this often means additional effort in the short term, the long term benefits of a large maintainable codebase by far outweigh the time put in when writing new code.

The following rules crimes and guidelines apply to all programming languages used (the guidelines combine JavaScript rules and Python best practice and coding crimes).

Recommendation¶

Call Parent Method For Zope Hook Method Override In Python¶

The real life example was on manage_beforeDelete for example in CopySupport.py. This method is used to uncatalog a document when it's deleted. In that case, never forget to call (Parent class).manage_beforeDelete(self, item, container).

Code Should Work If All Else Fails¶

It is a good idea to write code in such way that it has a consistent failover behaviour in case some assumptions are not met. For example, if a piece of code requires a regular expression which is obtained as a user preference, it is better to consider the case where the user forgot to enter the regular expression and provide a failover behaviour. If no consistent failover behaviour can be defined, then raising exceptions with clear message is the best thing to do.

A good practice is to raise an exception with a message stating "No regular expression was defined. The wrong way would be to throw an error.

Comments Should Be Used Wisely¶

Should be used with reason but include enough information so that a reader can get a first grasp of what a part of code is supposed to do.

Prevent Acquisition From Acquiring Restricted Properties In Python¶

The design of ERP5 is based on implicit acquisition just as most of Zope 2. However, ERP5 has its own way of considering acquisition. In ERP5, acquisition of methods is considered as a good thing while acquisition of document properties is considered as risky . This is why all properties defined in property sheets are set to 'None' at the level of the class or at the level of the PropertyHolder instance created by _aq_dynamic in Base class.

This way, acquisition is blocked on all properties defined by property sheets on portal types. If one wants to acquire a given property from a parent object or through a relation, this should be explicitly defined in property sheets. For an example, see Amount.py.

A good example would be acquiring methods through context or through containment, which is perfectly acceptable. It is used for example to access the current WebSection with the method getWebSectionValue in WebSection.py. It may be used also to display on a child object some properties which are defined on its parent. This is considered as normal in ERP5. The required use of accessors in ERP5 for getting and setting properties removes most risks related to implicit acquisition in Zope.

The use of acquisition for properties should therefore be considered only for implementation purpose and with great care. This following bad example (broken link) bug fix gives a very good example of what can happen if one does not care enough about property acquisition. Before this bug fix, 'getToolByName' was called on 'business_template' and therefore included 'business_template' in its acquisition chain.

The new business template which was created shortly after by 'newContent' would then also acquire 'business_template' properties. As a result, calling 'install' and 'uninstall' would install not only those items which were defined on the new business template but also all items for which nothing was defined on the new business template. This is because the way business template documents look up for business template items consists in accessing private properties such as '_message_translation_item' through 'getattr' rather than with an accessor.

This is perfectly acceptable in the context of ERP5 since such properties are considered as private or implementation related and we do not want to force implementors to use accessors at this level. However, it has risks. The risk in this case was to massively destroy installed scripts and forms in an ERP5 site in case of revert operation (and retrieve them later thanks to the portal_trash). The bug fix consisted in removing the acquisition context of 'business_template' by invoking 'getPortalObject()'.

To prevent further risk, we have set all private properties to None on the BusinessTemplate.py class with the following. This is a kind of application of the another principle, to write code that works even if all else fails.

Python Class Method Should Have Security Declaration¶

When writing a class from product that will be stored in ZODB, don't forget to declare appropriate security for methods. Usually you will use "Access contents informations" (for example here) to allow the user getting information from the object, or "Modify portal content" to allow the user modifying the object. For more information you can refer to Security chapter in the zope developer's guide, in the zope book or ERP5 5A Security Model.

The way to access content information is shown for example in the PasswordTool.py.

Use Of JavaScript Globals Is Discouraged¶

Variable declarations should always be made using var to not declare them as global variables. This avoids conflicts from using a variable name across different functions as well as conflicts with global variables declared by 3rd party plugins.

Good Example:

function sum(x, y) {
  var result = x + y;
  return result;
}

Bad Example:

function sum(x, y) {
  // missing var declaration, implied global
  result = x + y;
  return result;
}

Rule¶

A Non Anonymous JavaScript Method Must Be Declared Before Use¶

Non anonymous functions should be declared before use.

Good Example:

// [...]
function namedFunction() {
  return;
}
return {
  "namedFunction": namedFunction
};

Bad Example:

// [...]
return {
  "namedFunction": function namedFunction() {
    return;
  }
};

Abbreviations Are Not Allowed¶

Abbreviations should not be used to avoid confusion.

Good Example:

last_array_element_value = 1;

Bad Example:

l_val = 1;

Code Must Be Validated With JsLint¶

JSLint (http://jslint.com/) is a quality tools that inspects code and warns about potential problems. It is available online and can also be integrated into several development environments, so errors will be highlighted when writing code.

Before validating your code in JSLint, you should use a code beautifier to fix basic syntax errors (like indentation) automatically. There are a number of beautifiers available online. The following seem to be the best working:

• JSbeautifier.org
• JS-Beautify

Here, javascript sources have to begin with this header: /*jslint indent: 2, maxlen: 80, nomen: true */, which means it uses two spaces indentation, 80 maximum characters by line and allow the use of '_' as first variable name character. Other JSLint options can be added in sub functions if necessary; Allowed options are:

• ass: true if assignment should be allowed outside of statement position.
• bitwise: true if bitwise operators should be allowed.
• continue: true if the continue statement should be allowed.
• newcap: true if Initial Caps with constructor function is optional.
• regexp: true if . and [^...] should be allowed in RegExp literals. They match more material than might be expected, allowing attackers to confuse applications. These forms should not be used when validating in secure applications.
• unparam: true if warnings should not be given for unused parameters.

Good Example:

/*jslint indent: 2, maxlen: 80, nomen: true */
/*global jIO */
(function (jIO) {
  "use strict";
  var my_code = "is",
      perfect = "isn't it?";

  function doSomething(parameter_one) {
    return jIO.util.ajax({url: parameter_one});
  }

  // camel case tolerated for libraries
  window.myLib = {
    other_entries: "text",
    doSomething: doSomething
  };
}(jIO));

Bad Example:

var myCode = "is", perfect = "isn't it?";

var myLib = {
  otherEntries: "text",
  doSomething: function doSomething(parameterOne) {
    return jIO.util.ajax({url: parameterOne});
  }
};

Class And Id Attributes Are Lowercase With Underscore Separator¶

JavaScript can access elements by their ID attribute and class names. When assigning IDs and class names with multiple words, these should also be separated by an underscore (same as variables).

Good Example:

element.setAttribute("class", "my_class");
or
<div class="my_class" />

Bad Example:

element.setAttribute("class", "myClass");
or
<div class="myClass" />

JavaScript Closing Bracket Is At Indent Of Function Call¶

The closing brace should be on the same indent as the original function call.

Good Example:

function func() {
  return {
    "name": "Batman"
  };
}

Bad Example:

function func() {
  return {
           "name": "Batman"
         };
}

JavaScript Constructor Starts With Captial Letter¶

A constructor function starting with new should always start with a capital letter.

Good Example:

var test = new Application();

Bad Example:

var test = new application();

JavaScript Opening Brace Is On Line Of Current Statement¶

Always put the opening brace on the same line as the previous statement.

Good Example:

function func () {
  return {
    "name": "Batman"
  };
}

Bad Example:

function func() {
  return
  {
    "name": "Batman"
  };
}

Method Name Is Camelcase Starting With Lowercase Letter¶

A method/function should always start with a small letter.

Good Example:

function myFunction() {...}

Bad Example:

function MyFunction() {...}

Never Hardcode If Not Necessary¶

Hardcoded names or properties are hard to maintain and make code difficult to extend. Workflow state names for example should always use an accessor like shown below:

Good Examples:

if self.getSimulationState() in self.getPortalDraftOrderStateList():

Bad Examples:

if self.getSimulationState() == "draft":

Plural Is Not Allowed¶

Plural should not be used when assigning names or declaring parameters. Use _list or _dict instead.

Good Example:

var delivery_note_list = ["one", "two"];

Bad Example:

var delivery_notes = ["one", "two"];

Private Method Starts With Underscore¶

Private methods should use a leading underscore to separate them from public methods (although this does not technically make a method private).

Good Example:

var person = {
  "getName": function () {
    return this._getFirst() + " " + this._getLast();
  },
  "_getFirst": function () {
    // ...
  },
  "_getLast": function () {
    // ...
  }
};

Bad Example:

var person = {
  "getName": function () {
    return this.getFirst() + " " + this.getLast();
  },
  // private function
  "getFirst": function () {
    // ...
  }
};

Sessions Are Not Allowed¶

In our experience, sessions in Zope are buggy and incompatible with clustering. They generate a lot of ZODB conflicts too. Right approaches are based on the same concepts as the ones desribed in not accessing the temp folder across multiple requests.

Use Methods To Access Properties In ERP5¶

Always use accessors. Never access properties directly. ERP5 design is method oriented. All accessors can be automatically overloaded to support different data access schemes or extend a static property into a dynamic property (ex. price). All methods can also act in ERP5 as events through interaction workflows. This allows for implementing complex interactions and constraints. Not using accessors would lead to non generic code and potential inconsistencies with the general ERP5 system. Only the core of ERP5 may in some case access properties directly.

Good Example:

getter = self.getTitle()
setter = self.setTitle("foo")

Bad Example:

self.title

Use Of Id Attribute Is Not Allowed¶

"id" attributes should not be used in HTML element, because when working with gadgets (reusable compontents), duplicate Ids will break an application. For CSS selection, always prefer class attributes. For other HTML manipulation need, javascript should be used.

Good Example:

<style> .my_app > ul > li { ... } </style>
<div class="my_app">My App...</div>

Bad Example:

<style> #my_app > ul > li { ... } </style>
<div id="my_app">My App...</div>

Use Two Space Indentation ¶

Tabs and 2-space indentation are being used equally. Since a lot of errors on JSLint often result from mixed use of space and tab using 2 spaces throughout prevents these errors up front.

Good Example:

function outer(a, b) {
  var c = 1,
    d = 2,
    inner;
  if (a > b) {
    inner = function () {
      return {
        "r": c - d
      };
    };
  } else {
    inner = function () {
      return {
        "r": c + d
      };
    };
  }
  return inner;
}

Bad Example:

function outer(a, b) {
  var c = 1,
  d = 2,
  inner;
  
  if (a > b) {
  inner = function () {
  return {
  "r": c - d
  }}}
};

Crime¶

Cookies Are Not Allowed¶

ERP5 has been designed to be as much cookie technology independent as possible. In addition, cookies have many hidden limitations. For example, big size cookies are sometimes "scrambled" by Apache or Squid front ends. This means that a system based on the exchange of big size cookies can not be reliable (rfc2109 specifies a limit of 4096 bytes for a cookie). The right solution is, sadly, to use standard forms to exchange data. This may involve using encrypted binhexed or uuencoded data in hidden form fields.

FailUnless Should Not Be Used In Python Unit Tests To Test Identity¶

In unit test, use assertEqual, so we see the difference in traceback

Good Example:

self.assertEqual(self.getSimulationState(), "draft")
self.assertNotEqual(self.getSimulationState(), "delivered")

Bad Example:

self.failUnless(self.getSimulationState() == "draft")
self.failUnless(self.getSimulationState() != "delivered")

Never Access state_change Object In ERP5 Workflow Scripts¶

Even though it's likely to work you should instead use state_change['attribute_name'] as default access method.

Never Hardcode Interactions¶

Hardcoded interactions may require extensibility so use interaction workflows each time you have to implement something such as whenever X is called on A, invoke Y on B, even it may be a bit harder to debug at first.

Imagine that you want to call a method (eg. 'expand') each time an order or an order line is reindexed so that the simulation takes into account changes in that order. A simple (but wrong) approach consists in invoking expand from the overloaded reindex method defined on the Order or Order Line classes. The problem with this approach are multiple. First of all, there is no single point where this interaction pattern is explicitly defined. If there are a few interactions in the system, this is fine. But in an ERP system with many interactions, it leads to spaghetti code and no way to understand what happens.

Second, if one wants to extend the Order model and allow the user, let us say, to add a Resource instance inside certain orders, and at the same time a Rule which changes the invoicing based on parameters defined on that Resource instance, it is necessary to call 'expand' each time the Resource instance is modified. If we follow the simple approach, we must overload also the 'reindex' method of the Resource class (by creating for example a local Resource class with portal_classes). In the end, implementing interactions in a complex system may lead to overload most classes.

With interaction workflows, extending an interaction becomes much more simple. Adding an interaction definition for the Resource class each time the resource is part of an Order can provide a straightforward solution without having to overload any method.

Do not however overuse interaction workflows or create too many interaction workflows. It is better to have a couple of well defined interactions than dozens of small interactions. Also, if an interaction really does not need to be extended, it is acceptable to use interaction classes, decorators and/or method wrapper classes to achieve at the level of the python source code the same result as interaction workflows.

Never Hide Exceptions¶

Never hide exceptions to the user because exceptions may result in data inconsistency or system unstability and corruption. It is better to display an error to the user rather than finish a transaction and save corrupted data. At least, someone knows there is an error. Otherwise, errors happen, data is corrupted, and nobody knows. For example, make sure that SQLCatalog ZSQL methods generate exceptions and errors in the user interface in case of syntax error or database adapter connectivity problem.

Remember that hasattr hides all exceptions, including ZODB Conflict Errors, it's much safer to use getattr(obj, name, _marker)is not _marker in those situations.

As long as you re-raise the same exception, you can catch all types of exceptions.

Good Example:

try:
  ...
except:
  LOG(...)
  raise

Bad Example:

try:
  ...
except:
  pass

Never Modify 100 Objects In A Transaction¶

If you need to modify large amounts of Zope objects (more than 100), please use activities. Modifying too many objects in a single transaction leads to conflicts which degrade the performance of the Zope application server and eventually make applications unusable.

Never Name A Base Category The Same As A Property In ERP5¶

Base category ids and property ids are meant to form a global vocabulary where every id is defined once and once only. This requires base categories to have a different id from properties. The same is true for base domains (which must be named with an id which is neither a category id or a property id).

Never Override Python Builtin Names¶

Pay attention to not override builtin python names (as returned by dir(__builtins__) in the Python interpreter) as it makes debugging harder and hides errors.

If you do so, the benefit is that you can use some python code checkers like PyFlakes, pylint or PyChecker.

Never Store Calculation Result In Python Property¶

Always use properties to store document 'original' content (ie. the one which was entered by the user). If you need to process document original content and store the result somewhere, either make this is a new document (and Simulation should be used if possible to track relations between calculated documents and original documents), or this is a workflow variable (with complete historisation), or this is an internal property to be handled within the class code.

Never Use aq_parent To Get Document Parent In Python¶

Use the getParentValue() method instead of aq_parent, which is defined on Base class. aq_parent is only useful if you wish to get the acquisition context of an object, which may be different from its physical container. For example, aq_parent is useful to calculate a breadcrumb whenever the acquisition path results from the combination of multiple physical path (ex. web_site_module/a/b/web_page_module/c). Refer to the Zope Developer Guide for more information on acquisition.

Never Use ContentValues or ObjectValues On More Than 50 Documents In ZODB¶

objectValues and contentValues are suitable for folders with a few documents (ex. less than 50). Many folders in ERP5 may include millions of documents. Use searchFolder if you need to list or sort documents in such folder.

Never Use DoActionFor In ERP5¶

Never use doActionFor to implement logic. ERP5 generates transition methods for all workflow transitions implemented as workflow methods. If you need to programmaticaly trigger a transition on a workflow, use transition methods. Transition methods are intended to represent the workflow logic, unlike workflow actions which should be used for user interface only.

Good Examples:

context.submit()

Bad Examples:

context.getPortalObject().portal_workflow.doActionFor(object, 'submit_action')

Never Use GetPath To Retrieve A Url In ERP5¶

Use absolute_url_path instead or absolute_url, because getPath only returns the path of an object in the OFS. This means the physical path. Whereas absolute_url_path takes into account all virtual hosting configuration. absolute_url includes the host part of the URL (http://hostname/url/to/object) while absolute_url_path only returns the user URL (/url/to/object).

Never Use Python If Obj if Obj Is Not A Simple Type¶

Truth value testing on Zope / ERP5 objects has surprising behavior. For example "if obj" returns false if the object is an empty folder, and "if obj" returns true if it is a folder with sub objects. This is because python uses __len__ if __nonzero__ method doesn't exist on object. So "if obj" MUST NOT be used to check if the object is None.

Good Example:

document = context.getFollowUpRelatedValue(portal_type='File')
if document is None:
  document = ...

Bad Example:

document = context.getFollowUpRelatedValue(portal_type='File')
if not document:
  # Here document may exist, but the conditional is False as document doesn't have sub-objects

Never Never Use Reindex In ERP5¶

Sometimes, it is possible that you feel that it will be easier if you can use immediateReindexObject or recursiveImmediateReindexObject. Theses two methods must be used only for debugging. So never use it in any script, never use it in the ERP5 Code.

Each time we use immediateReindexObject, it will work for development, but it will fails on production if you have more than 2 users.

If you need immediateReindex, this probably means that you are doing the wrong way.

Never Use Temp ZODB Folder Across Multiple Requests¶

Storing into temp folder is incompatible with ZEO and clustering, unless this is only for a single transaction. If you need to share data over multiple transactions, either user ERP5 preferences, an ERP5 selection object or any ad hoc storage. ERP5 selection object implementation is worth having a look at because it has been implemented to reduce ZODB conflicts by hashing selections per use. Make sure though that all changes to ZODB storage happen within the same mounting point of ZODB (ex. /erp5/portal_selections). This allows for selective compacting ZODB from time to time or for using different ZODB backends whenever complete history and traceability is not needed.

Never Write To ZODB If Not Required¶

Each write transaction into the ZODB creates risks of conflicts and increases the size of the ZODB. Unnecessary writes can this way easily increase a 100 MB database into a 10 GB database within a short time.