With the new database abstraction layer introduced in Drupal 7, chained method calls have become more common. I've seen different styles used within different modules and it would be nice to create a standard around it.

The style below is what I prefer and I've seen often in core:

$query = db_select($table, 't')
  ->fields('t')
  ->condition('entity_type', $entity_type)
  ->orderBy('delta');
$results = $query->execute();

With the style above, you can easily tell that fields(), condition() and orderBy() are chained from the result of db_select().

I'm not sure if there are any examples in core where the chain diverts to another object, but I feel that something like the following would be appropriate:

// I could pass the fetch mode to fetchAll(); I'm just trying to demonstrate.
$results = db_select($table, 't')
  ->fields('t')
  ->condition('entity_type', $entity_type)
  ->orderBy('delta')
  ->execute()
    ->setFetchMode(PDO::FETCH_ASSOC)
    ->fetchAll();

Again, this style makes it easy to see the chaining and also makes it clear where the tree diverges to another object. If the chain somehow returns back to the original object, a chained method call from that result would then be flush with the call to execute().

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: +Coding standards

tagging

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
CreditAttribution: ianthomas_uk commented

Similarly, where should trailing brackets be placed when you pass a long parameters. For example, my preference is:

$query = db_select($table, 'paints')
->condition(db_or()
  ->condition('color', 'red', '=')
  ->condition(db_and()
    ->condition('color', 'blue', '=')
    ->condition('shade', 'dark', '=')
  )
);

Here each trailing bracket gets its own line, indented by the same amount as the line that opened the bracket. I have also seen people put all the trailing brackets on the same line, and then jump back several steps in the indenting in one line. I'm not sure if there is an existing Drupal convention that would encourage one style over the other. The closest I could find was arrays, which are laid out in a similar style to my above example.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

A good place for this is Object-oriented code under "Chaining".

The suggestion to make additional indentations when different objects are returned is interesting, but this is currently not used in core. Wouldn't it be a better start to just document how it is widely done in core atm: just indenting all subsequent lines with 2 spaces, regardless of the returned object?

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Issue summary: View changes

Fixed sentence that was not properly put together.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Project: Drupal core » Drupal Technical Working Group
Version: 8.0.x-dev »
Component: documentation » Documentation
Issue summary: View changes

Coding standards decisions are now supposed to be made by the TWG

tizzo’s picture

tizzo CreditAttribution: tizzo commented
Project: Drupal Technical Working Group » Coding Standards

Moving this issue to the Coding Standards queue per the new workflow defined in #2428153: Create and document a process for updating coding standards.

Niklan’s picture

Niklan
Primary language Russian
Location Russia, Perm
CreditAttribution: Niklan as a volunteer commented

I also think it should explicitly describe or propose a best practice for chaining method calls, at least at the documentation page.

https://www.drupal.org/node/608152#chaining:

In the case where you have a fluent interface for a class, and the code spans more than one line, the method calls should be indented with 2 spaces:

$query = db_select('node')
  ->condition('type', 'article')
  ->condition('status', 1)
  ->execute();

But it doesn't specify when a chained call should start from a new line. This implies that following examples are also valid ones:

This one doesn't pass 80 char limits and chop down before it.

$query = db_select('node')->condition('type', 'article')->condition('status', 1)
  ->execute();

This one keeps the first chained call at the same line and the rest is from the new line.

$query = db_select('node')->condition('type', 'article')
  ->condition('status', 1)
  ->execute();

I also was confused should I keep the first call on the same line and chop down the rest, or should I chop down from the first one. This is unclear from documentation, you can only assume that chopping down from the first call is more preferable, but there is no any mentions of this.

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented

I take that, as long as the line is not longer than 80 characters (or the new limit Drupal coding standards sets) and it is readable, both the following code examples are fine.

$query = db_select('node')
  ->condition('type', 'article')
  ->condition('status', 1)
  ->execute();

$query = db_select('node')->condition('type', 'article')
  ->condition('status', 1)
  ->execute();

Personally, I prefer the second example, simply because the function returning the object used for method concatenation is on the same line with the first method call. That does not mean the first example is wrong.