edit

Coding standards

The coding standards described here have been written years after the start of the project. They describe the most common patterns and best practices for writing CiviCRM code, and they seek to answer the question, "what is the proper way of doing X?" They cover not only whitespace and appropriate styles for writing the code itself but also how to document code at file, function, line and other levels.

Developers are encouraged to take these standards as an expression of "what we want" more than of "what we have everywhere." This means that if you find yourself working on a piece of code that doesn't follow these standards, you are encouraged to re-factor it so that it is. IRC is a good place to come if you have questions or need clarification.

Quite a few blog posts have been written over the years in the architecture series the oldest one describing the DAO BAO/ Templating and file structures should still be on your must read list. Eileen's post from July 2013 is also a keeper. Please refer to it as a good overview of how to do things in a standards-compliant way 4.3+.

CiviCRM vs Drupal

In general, CiviCRM follow's the Drupal Coding Standards, but we have some minor modifications which are noted specifically in the other pages in this chapter.

Continuous integration

Jenkins will automatically check all pull requests for coding standards conformance, and code which does not meet the standards will not be merged.

Tools

If you have a development site with buildkit you can use Civilint to check your code against CiviCRM's coding standards.

You can also set up your IDE to lint your code.

Improving code conformance

If you find code that is not meeting the standards, we encourage you to improve it! But please follow these guidelines when doing so:

  • Isolate your coding standards improvements into commits which do not contain otherwise unrelated changes.

Orange Light code

Orange light code is code that has the feel that it is wrong and should be refactored. Developers who are looking to contribute to this effort may want to consider doing any amount self-contained work on the following code.

  • Using joins on civicrm_option_value rather than using PseudoConstants - this has performance iplications
  • Removing as much as possible passing by reference to functions.
  • Increasing code complexity, this has two issues firstly it increase the fragility of the code and also makes it harder to test
  • Where ever $session = CRM_Core_Session(); $userid = $session->get('UserID'); or very similar these calls should be replaced with CRM_Core_Session::singleton()->getLoggedInContactID();
  • Replace CRM_Core_Resources->singleton()->add... and similar with Civi::resources()->add...
  • Clean up messy code, see if code can be refactored and moved into parent classes and make them more generic, Also elimiate any duplicate code
  • Increase the usage of Doc blocks to help with auto generation of code
  • Move more business logic out from the Froms and API if possible to the BAO level
  • Remove eval() instances
  • Develop & confirm standards listed above (incl decide on smarty coding templates, separation of tpl and js)
  • Move business logic from the forms to the BAO
  • Cleanup and centralize the token code.
  • Code duplication should be addressed - e.g the introduction of a select function in CRM/Report/Form.php around 3.3 made identical functions in child classes obsolete. This is a good taks for someone wanting to learn
  • Look at repurposing the coder module to keep CiviCRM tidy
  • Git rid of unused functions
  • Look through the "todo" "fixme" and "hack" comments and see about fixing them
  • As CiviCRM now requires PHP 5.3, modify code to use the Drupal 8 coding standard of using the PHP keyword const in place of define();

In CiviCRM we aim to compile SQL in the following order of preference

  1. API - Predicatble well tested
  2. DAO / BAO - CRM_Core_DAO()->fetch() CRM_Core_DAO->copyValues()
  3. CRM_Utils_Select
  4. Compiled SQL

In CiviCRM we aim to use the APIs as much as possible as they have a solid test framework and a test contract behind them. This means if something is tested through the phpunit tests then it is on CiviCRM's responsiblity to ensure that does not break.