Coding_Conventions

General coding guidelines for contributing

Test your code

As our codebase grows and evolves, we need to be sure that features are still working as expected, and that new code does not introduce any breaks in the code.

Please do write tests for any new code you apply, and feel free to contribute any missing tests for existing code—we would appreciate the help,

Read our Testing conventions guide for more information.

Coding style

We realize that every developer has their own style and we encourage a bit of individuality. However, we firmly believe that it's easier to work on code when everyone follows similar conventions and styles.

Aiming to maintain a consistent coding style throughout the codebase, we decided to use EditorConfig.

EditorConfig consists in a single file, include at the root of the project where rules are added to automaticaly maintain the coding style. Most of the IDEs have plugin or native support for EditorConfig. You can find the list of supported editors on the official website.

Some basic rules have been activated for most of the files in the project :

• Idents should use two spaces
• A new line is inserted at the end of each file.

Remove trailing whitespace

Please do not submit code for review that has trailing whitespaces. This adds a lot of noise to git commits, and is wasteful.

Most text editors can be configured to remove trailing whitespace automatically.

Wrap columns at 90 characters

If a line is going to go beyond 90 characters please break it out onto multiple lines.

Many developers work on narrower, vertical, or split screens. Keeping your code within 90 characters makes it easier to read, and to compare diffs using Git.

For example:

Bad

users = [{email: @user.email, password: 'bAd_pas$word1', remember_me: true}, {email: 'unknown@institution.org', password: 'password123', remember_me: true}]

Good

users = [{email: @user.email, password: 'bAd_pas$word1', remember_me: true},
         {email: 'unknown@institution.org', password: 'password123', remember_me: true}]

Check your code using Rubocop

We've installed Rubocop in the app with some sensible modifications made to the standard Rubocop style guide.

You can read through the standad Rubocop style guide here: http://docs.rubocop.org/en/latest/cops/

And you can view the rationales behind our own preferences, where different, here: https://github.com/dmproadmap/rubocop-dmp_roadmap/tree/master/config/

Please check your code against our style guide before you commit a new PR. You can run Rubocop via a bash shell via:

# The -p argument will run checks faster on devices with multi-core processors
$ rubocop -p

Most text editors can be configured to run Rubocop style checks as you write your code, if not you can install external plugins such as the [solargraph] gem (https://github.com/castwide/solargraph).

Fix any files you've changed

You will probably notice a lot of existing code does not yet conform to our style guide. We're currently trying to move towards full complicance, but this is is not an immediate process.

Please assist us in this by running Rubocop checks on any files you've changed, and committing these as a separate commit.

The following executable will run Rubocop only on the files changed on your branch.

$ bin/rubocop_changed

Documentation

If you'd like to understand the code better, you can generate HTML documentation for the current codebase by running the following rake task:

$ rake doc:app

This will create a directory filled with automatically generated HTML at ./doc. Open the index file there to view the application documentation.

$ open ./doc/index.html

Document your code

Please provide documentation for code you add by using Ruby comments. This is not essential in every instance, but encouraged—particularly when the code is complex.

Specifying the return values of each method is particularly useful, as it lets other developers better understand the contract of the Object they're working with.

Use TomDoc

Use the TomDoc syntax to document your code. This is the most lightweight and readable documentation syntax. It's better for you, and for us.

Documentation is generated using the YARD gem with the TomDoc plugin.

Comment within methods where the code isn't clear

Add comments inside your methods where you feel the code isn't clear. Keep comments terse.

Bad

def sum_ids
  # Pluck out all of the ids of associated records in an array
  ids = records.pluck(:id) 
  ids.sum
end

Good

def sum_ids
  # Array of all records ids.
  ids = records.pluck(:id)
  ids.sum
end

Add comments above the line(s) of code, not on the same line

Bad

def sum_ids
  ids = records.pluck(:id) # Array of all records ids.
  ids.sum
end

Good

def sum_ids
  # Array of all records ids.
  ids = records.pluck(:id)
  ids.sum
end

Update existing documentation

When you change a piece of code, update the existing documentation where necessary. Please do not leave redundant documentation in this shared codebase.

Remove unwanted code

If code is no longer necessary, please delete it. If needed, it can be retrieved from source control at a later date. Do not comment-out unwanted code.

Architecture

Extract complex view code into helper methods

Avoid writing HTML code in your views with complex conditionals or variable assignments.

Where possible, extract these instances out into meaningful helper methods.

Bad

<!-- users/show.html.erb -->
<h2>
  <%- if @user.name? %>
    <%= @user.name %>
  <% else %>
    Unknown user
  <% end %>
</h2>

Good

# users_helper.rb
def header_for_user(user)
  if @user.name?
    @user.name
  else
    "Unknown user".freeze
  end
end

<!-- users/show.html.erb -->
<h2><%= header_for_user(@user) %></h2>

Write rollback-able migrations

When writing database migrations, ensure they can also be rolled back via the $ rake db:rollback command. This is essential when working across various branches and pull-requests.

Support MySQL and PostgreSQL in all queries

In the few cases that it's necessary to write your own custom SQL queries, please ensure your code works on both MySQL database and PostgreSQL.

You can test this by specifying the database adapter when running tests.

$ DB_ADAPTER=mysql2 rspec spec
$ DB_ADAPTER=postgresql rspec spec

Extract RESTful resources

When modifying the controller layer, keep controllers lean and focussed on handling a single resource. Bloated controllers increase the cost of maintaining a codebase.

When adding new actions and routes, consider whether you're defining a new domain concept or abstraction. A good hueristic to test this is to ask: Is this action one of the 7 standard RESTful actions? (index, new, create, show, edit, update, destroy). If not, the chances are you're defining a new domain concept and resource that warrants its own controller.

Bad

class PlansController < ApplicationController
  
  def request_feedback
    # ...
  end
end

Good

class FeedbackRequests < ApplicationController
  def create
    @plan = Plan.find(params[:plan_id])
    # ...
  end
end

See Open-closed principle, below.

Observe SOLID design principles

Observe SOLID design princples when writing code. These are tried and tested principles for writing reliable code that is easy to maintain over time.

If you haven't heard of SOLID before, or need a refresher, this talk by Sandi Metz is great: https://www.youtube.com/watch?v=v-2yFMzxqwU&frags=pl%2Cwn

Single responsibility principle

Don't give objects too many jobs to do. This leads to tightly coupled code that's difficult to test and to change in future. Where possible, break objects into smaller objects that deal with discrete aspects of the overall domain.

Open-Closed principle

Once your code has been written, it should not require modification unless to fix bugs. Strive to write code in a way that components are as atomic, extensible, and interchangable as possible.

Listkov

Write your code in a way that any instance of a class may be substituted by any instance of one of its subclasses.

Interface segregation principle

Write your code in such a way that objects are not cluttered with methods they will not directly use.

Bad

class User
  attr_reader :name, :age, :admin_permissions
  def initialize(**attributes)
    @name              = attributes[:name]
    @age               = attributes[:age]
    @admin_permissions = attributes[:admin_permissions]
  end
end

class Admin < User
end

class NonAdmin < User
end

Good

class User
  attr_reader :name, :age
  def initialize(**attributes)
    @name = attributes[:name]
    @age  = attributes[:age]
  end
end

# Only give the admin class the permissions, if that's the only place it's needed
class Admin < User
  attr_reader :permissions
  def initialize(**attributes)
    super
    @permissions = attributes[:permissions]
  end
end

class NonAdmin < User
end

Don't Repeat Yourself

DRY (Don't Repeat Yourself). Please always look through the existing code to make sure you are not reinventing something that has already been done. Also, try to write code that's as reusable as possible and avoid copying and pasting is always appreciated.

Ahdere to 12-Factor tenets

The 12-Factor App tenets help us create an application that's easy for others to configure and deploy on a variety of servers.