Coding Style Guidelines

PHP

Coding Style

From version 5.7 and onward, concrete5 is adopting the PHP Framework Interoperability Group's PSR-2 coding standard. This standard dictates a number of things, including spaces vs. tabs, brace placement, method naming, and more. Please read it and adhere to it.

Tough Decision

Like a lot in life, coding standards are deeply personal. I myself strongly disagree with several decisions in this standard (and, to spare myself the inevitable arguments, I'll leave which these are to your imagination.) That said, there is a lot of good in here, and I think it can only help the ecosystem. So I, too, will bite the bullet.

We're in the same boat, people. Let's set sail!

Keeping Up With Development

While concrete5 is being developed, some of the stuff done in the past might become deprecated or old convention. Please refer to the following document in order to keep your codebase up to date:

Deprecated Code Reference (ongoing)

File Naming and Location

From version 5.7 and onward, concrete5 is adopting a modified naming standard based on PHP Framework Interoperability Group's PSR-4. Our built-in autoloader will automatically find classes that adhere to this system.

All core libraries adhere to PSR-4 with no modifications. What does that mean? That means that our namespaced core libraries like the Page class

\Concrete\Core\Page\Page

Can be found at

concrete/src/Page/Page.php

PSR-4 basically states that projects can map different namespace prefixes to arbitrary starting directories. That's what we're doing.

If you are writing code that belongs in the core source directory, you must adhere to PSR-4 as is. Classes must be capitalized, etc...

Modifications

For classes that do not exist within the src/ directory (such as block controllers, attribute controllers, etc...) We ask that you name your controllers with the same capitalization that PSR-4 requires, but you name your files with concrete5's classic lowercase + underscore method. These files will be converted by camelcasing those filenames on the fly.

What does that mean? The blocks directory and its contents still looks the same as concrete5 before 5.7. For example, the Page List block's controller is still located at

concrete/blocks/page_list/controller.php

If you were strictly following PSR-4, you would need to name your class

controller

inside the name space

\Concrete\Block\page_list

This is ugly and causes other problems. So instead, when we request the class

\Concrete\Block\PageList\Controller

We first check to see if

concrete/blocks/PageList/Controller.php

exists (since that is the default PSR-4 autoloading behavior.)

If it does not (and it won't, for blocks, attributes, single page controllers, etc...) – then we uncamelcase using our own methods, starting at the item after blocks. Backslashes become directory separatores, and camelcasing becomes underscores.

concrete/blocks/page_list/controller.php

And that's how we handle non-library classes in concrete5 5.7.

Inversion of Control (IoC)

Concrete5 uses a concept called "inversion of control". Through this, we are able to access different classes centrally from one place which makes swapping or overriding specific functionality much easier for developers.

This makes the classes available throght the inversion of control container (IoC container) which is referenced as $app in many places of the core. Through this class, you are able to initiate new classes easily using the $app->make() and $app->build() methods.

For example, if we want to access the database connection, we could call $app->make('Concrete\Core\Database\Connection\Connection'). Or if we want to initiate a new instance of the logger class, we could call $app->build('Concrete\Core\Logging\Logger').

Define Class Dependencies In Constructor

Because of the IoC concept concrete5 uses, we are able to take advantage of this to make our code cleaner. This makes it possible for developers to define the class dependencies in the class constructor and the container will resolve them automatically through the IoC making it easy for developers to get access to these classes and also to other developers to override them.

In the past this was not as straight forward because of which in the core and 3rd party code, we still see a lot of code using the $app container class directly or even code using the old style of accessing it through facades (Core::).

Here's an example of what some old style code would look like (Incorrect) and what it should look like (Correct).

Incorrect:

<?php
namespace Application\SomeStuff;

use Concrete\Core\Support\Facade\Application;

class MyClass
{
    public function doSomething()
    {
        $app = Application::getFacadeApplication();
        $db = $app->make('database')->connection();
        if ($db->tableExists('Areas')) {
            $this->doSomethingElse();
        }
    }

    // ...
}

Correct:

<?php
namespace Application\SomeStuff;

use Concrete\Core\Database\Connection\Connection;

class MyClass
{
    protected $db;

    public function __construct(Connection $db)
    {
        $this->db = $db;
    }

    public function doSomething()
    {
        if ($this->db->tableExists('Areas')) {
            $this->doSomethingElse();
        }
    }

    // ...
}

With this type code, you will need to initiate your class through the IoC container for it to resolve the dependencies automatically:

<?php
$my = $app->build('Application\SomeStuff\MyClass');
$my->doSomething();

Existing Code

We have already taken care of moving files to support these updated conventions. For coding style (including spaces vs. tabs, braces, etc...) we will be modifying existing core code to support this code base as needed. This is not a high priority, but we will accept pull requests quickly that modify this code for us. As for now, most of the codebase has been converted to follow these conventions.

Important Note

If you submit a pull request for this type of cleanup, please keep each request to one file – for the sake of vetting your pull request. Please do not run the entire concrete5 source through a cleanup tool and send that to us. We could do that ourselves ;-)

Templates (HTML)

There are a couple of general rules to follow with your presentational layer (templates, files containing HTML):

  • Keep your presentational layer as clean as possible.
  • Indent your HTML with spaces and use 4 spaces per indent.
  • Do not use complex control structures (if, else, loops)
  • Assign as much of your variables outside of the presentational layer as possible (usually in controller)
  • Separate repeating elements to their own elements.
  • Keep logic out of the templates

Some of these points are clarified below.

Assigning Variables

Incorrect:

<?php defined('C5_EXECUTE') or die("Access Denied.");
$name = $object->getName();
$price = $product->getBasePrice() + $vatPercentage * $product->getPrice();
?>

<table>
    <tr>
        <td><?php echo t("Name") ?></td>
        <td><?php echo $name ?></td>
    </tr>
    <tr>
        <td><?php echo t("Price") ?></td>
        <td><?php echo $price ?></td>
    </tr>
</table>

Correct:

<?php // controller
// ...
class Controller
{
    // ...
    public function view()
    {
        $this->set('name', $this->object->getName());
        $this->set('price', $this->product->getBasePrice() + $this->vatPercentage * $this->product->getPrice());
    }
    // ...
}
<?php // template ?>
<?php defined('C5_EXECUTE') or die("Access Denied."); ?>

<table>
    <tr>
        <td><?php echo t("Name") ?></td>
        <td><?php echo $name ?></td>
    </tr>
    <tr>
        <td><?php echo t("Price") ?></td>
        <td><?php echo $price ?></td>
    </tr>
</table>

Keep logic out of templates

Make sure your template code is as clean and understandable as possible. Keep logic out of the template and focus on the presentational aspect. In case you have repeatable elements in your template, please use the elements concept.

Incorrect:

<?php defined('C5_EXECUTE') or die("Access Denied."); ?>
<?php
function print_list_item($item) {
    echo '<li>' . $item->getName() . '</li>;
}
?>

<p><?php echo t("Check out our items:") ?></p>
<ul>
    <?php foreach ($items as $item) : ?>
        <?php print_list_item($item); ?>
    <?php endforeach; ?>
</ul>

<p><?php echo t("Check out our products:") ?></p>
<ul>
    <?php foreach ($products as $item) : ?>
        <?php print_list_item($item); ?>
    <?php endforeach; ?>
</ul>

Correct:

<?php // template code ?>
<?php defined('C5_EXECUTE') or die("Access Denied."); ?>
<p><?php echo t("Check out our items:") ?></p>
<ul>
    <?php foreach ($items as $item) : ?>
        <?php View::element('element_scope/list_item', ['item' => $item]); ?>
    <?php endforeach; ?>
</ul>

<p><?php echo t("Check out our products:") ?></p>
<ul>
    <?php foreach ($products as $item) : ?>
        <?php View::element('element_scope/list_item', ['item' => $item]); ?>
    <?php endforeach; ?>
</ul>
<?php // elements/element_scope/list_item.php ?>
<?php defined('C5_EXECUTE') or die("Access Denied."); ?>
<li><?php echo $item->getName() ?></li>

Generally the given example would be "too simple" case to extract the list item to its specific element file. It is just used as an example that if you have repeable elements, you can use concrete5 elements.

Note

While this is the convention to aim for, the core itself is not perfect either. concrete5 has existed almost 20 years and some of the codebase has lived along for a very long time. The system has many users and sometimes it might not be that simple to update a piece of code that is widely used.

JavaScript

Coding Style

JavaScript should use the Airbnb JavaScript Style Guide, with the following modifications:

  • Use four spaces for indention, for tool consistency and consistency with existing core code.
  • When saving a reference to "this," use "my" instead of "_this."

File Naming and Location

JavaScript file names should be all lowercase, with dashes the only non alphanumeric character allowed in the filenames before the extension.

Yes

  • js/color-picker.js
  • js/core/jquery-ui.js

No

  • js/ccm.whatever.js
  • js/MyFile.js
  • js/who_knows.js

JavaScript should go in the standard js/ directories only. Unless it is explicitly allowed by the core (e.g. view.js files in blocks.)

Existing Code

While the style of our JavaScript is improving massively in 5.7, many of our file still need cleanup, and our JS naming definitely doesn't currently support these standards. We will be handling the renaming of files as needed. But we would love to have help with the style of the code itself.

Important Note

Again, if you submit a pull request for this type of cleanup, please keep each request to one or just a few files – for the sake of vetting your pull request. Please do not run the entire concrete5 source through a cleanup tool and send that to us. We could do that ourselves ;-)

CSS/LESS

Coding Style

Your CSS/LESS is largely your business, but we do ask that all selectors be lowercased, with dashes as their delimiters. If coding something for the core, please prefix it with 'ccm-' and try and be consistent.

File Naming and Location

CSS file names should be all lowercase, with dashes the only non alphanumeric character allowed in the filenames before the extension.

Yes

  • css/jquery-ui.css
  • css/redactor.css
  • css/conversations.css
  • css/jquery-rating.css

No

  • css/MyLib.css
  • css/_whozits.css
  • css/ccm.spellchecker.css

CSS should go in the standard css/ directories only. Unless it is explicitly allowed by the core (e.g. view.css files in blocks.)

Existing Code

Most of our CSS has been moved into LESS files as of 5.7. We will accept pull requests for minor syntax changes here and there, but we're generally pretty pleased with this cleanup so far, and it doesn't require too much.

The names of our CSS files definitely don't currently support these standards. We will be handling the renaming of files as needed. Please do not submit pull requests renaming core files.

Important Note

Again, if you submit a pull request for this type of cleanup, please keep each request to one or just a few files – for the sake of vetting your pull request.

Further Discussion

These decisions have not been made lightly, and they're probably going to upset some (including some core developers – gulp). But hopefully this can lead to a cleaner, more uniform concrete5 source.

For more discussion on this, including any addendums, additions or fixes, please post in the Documentation Forum.

Loading Conversation