Implementing code style in an existing project

Probably, in any team, sooner or later the question arises of creating and approving coding standards. At first glance, the task is quite trivial. But only in the case when it is decided at an early stage of the project development. Then, developers just need to select and apply one of the popular standards, and most often this framework is made for them by the framework that they use.

In our case, everything is not so simple. The project we are working on began its life even before the various standards and descriptions of best practices in the development environment began to pay much attention. In particular, long before the emergence of the currently popular PSR standards for PHP. For these reasons, the task of standardizing the code was not set at earlier stages, but now it has appeared to our team as a challenge.

In this publication, we will talk about how we came to understand the need for a single code style, and developed methods for its gradual introduction into a large-scale project. This experience may be of interest to those who are not using standardization yet, but already feel the need for it.

What we have

• Base code (PHP, MySQL, HTML, SCSS, JavaScript), formed over 10 years;
• The tacit understanding of the general format, which include:
  - snake_case for the names of variables and functions in the backend;
  - basic formatting rules (indents, spaces, hyphens);
  - the rules for writing sql queries;
  - names of tables and columns;
  - Hungarian notation for variables containing input data.
  These are just a few examples of the nuances of which there is a huge variety in each project.

What we do not have?

Officially adopted style, recorded on paper and binding. Perhaps that is why the unofficial format is not constant and changes with the passage of time - its “evolution” is easy to follow with the help of the version control system.

Awareness of the problem

At a certain point, we began to notice that we quite often discuss code in the context of formatting, and this takes a lot of time. At the same time, the team began to actively expand. Our new colleagues had their own habits in writing code and were not familiar with those practices that are well established in our team. As a result, their style of writing code was in conflict with our “unspoken rules”, having a significant impact on productivity.

For example, the combination of different formatting styles and the use of language constructs within a single code block led to a significant decrease in its readability:

for ($i=0; $i<$num; $i++) { if (in_array($items[$i]['value'],$filtered)) continue; array_push($valid, $items[$i]); if(!$items[$i]['in_menu']) continue; $in_menu[] = $items[$i]; } 

But if within the team there is always the opportunity to promptly discuss and resolve controversial issues, then when working with third-party developers there are much more difficulties. We involved third-party specialists in the development of the client part, and it was in the frontend code that the greatest variation in styles appeared. For example, in the naming of classes in a single block of HTML code, different notations and even different implementations of BEM could be combined.

But here is the main difficulty that arose when collaborating with experts from the outside: how to maintain code written using practices and approaches that not all project developers own?

Inconsistent choice of methodology in the layout and subsequent editing sometimes led to the following:

 <div class="i-article__item__item__content">...</div> 

or

 <div class="i-article__about-article-block_dropdown-handler">...</div> 

If in the first case BEM is traced, then in the second case it is impossible to understand what is happening. In addition, the use of BEM in naming classes in HTML sometimes did not at all entail advantages in CSS. The class from the first variant, which contains the description of the nesting of the element, was still described using nested styles in CSS:

 .i-article__item .i-article__item__item .i-article__item__item__content { ... } 

How could this happen? An example from experience: the remote layout designer did his work partially, without having managed to complete the mobile version of the interface. Another specialist joined the work, adding a new code in his “unique” style. At the next stage, when porting the layout to the dynamic engine, it became necessary to correct something quickly, and this was done by the backend developer, who, naturally, did not delve into the “subtlety” of the format used by the layout designers.

It became obvious that one of the highest priorities for us is to avoid similar discrepancies in the future. Delaying the decision could result in a loss of productivity and an increase in development time. The team finally formed an understanding of the need for approval of a single code format.

Decision making

Since the amount of code is measured in hundreds of thousands of lines, it was not possible to find and rework all instances of style violation, even with the use of automatic tools. Therefore, we decided to start using the same style for the new code, while at the same time updating small fragments of the old code related to the task being performed.

Then it was necessary to decide on the format itself. It turned out that the team members had significant differences in their views on the “right style”: some preferred camelCase, others insisted on using snake_case in everything, even in client-side JavaScript.

In this case, the level and background of the developer plays a big role. As you may have guessed, in the second case we are talking about a long-term backend programmer.

Disagreements arose on many points, but they agreed on one thing unanimously: I didn’t want to use one of the existing style guide “as is”, it is necessary to preserve the unique style of the project. Therefore, it was decided to document the fact that it was used behind the scenes, and to submit non-obvious and controversial points for discussion.

For a rather short period, we have formed our own style guide for php, sql and partly JavaScript code. Next in line were CSS and SCSS.

The greatest number of developers, including remote ones, contributed to the formation of CSS code, and CSS can be said to have a different liberty ... It soon became clear that following a single style is possible only with the most accurate and detailed description of all the rules, with a multitude of examples of how and how not to do it.

The discussion dragged on, and the team could not reach a consensus on a number of points. As a result, for SCSS and CSS we decided to apply a ready-made third-party document with our small adjustments.

We used Sass , CSS , and familiarization and approval of amendments took the team no more than 3 hours.

Performance control

To monitor compliance with the accepted rules, we decided to use the means of automatic verification. Today, there is a fairly large selection of similar tools, many of which work in the format of cloud services and are easily integrated with popular platforms for hosting code and continuous integration.

The choice fell on SonarQube - a static code analysis platform that supports various programming languages ​​and offers several formats of use, including selfhosted, along with the cloud version (SonarCloud). In addition, there is integration with our platform CI (Teamcity) and the ability to pre-check in the code editor (SonarLint).

The installation process along with the plug-in package took no more than half an hour; integration with Teamcity took just over an hour. We had to create so-called "quality profiles" - a set of rules for each language used during the test. Otherwise, the result of the analysis would not reflect the real picture.

It took about three hours to set up profiles for our preferences. Interestingly, for PHP, there are 3 options offered by default: “PSR2”, “Drupal” and corporate “Sonar way” - many are likely to be able to use the system “out of the box”.

After installation and configuration, the “moment of truth” came - a test of checking the quality of the code against the formulated rules. Run the sonar-scanner command and wait. It took about 4 minutes to index files and check more than 500 thousand lines of code. The result of the analysis is available in a rather nice and convenient interface:

The presented result was for us an expected consequence of the introduction of the code style and automatic controls at a late stage of the project development. In further work with this tool, we will be most interested in the indicators on the right side of the screen, highlighted in yellow. This data reflects the analysis of the new code added since the previous assessment.

What's next?

Our goal is to control the quality of the new code and gradually improve the overall statistics presented on the left side of the screen. Within two weeks, we will check the system in “field conditions” and estimate the benefits of its implementation. In the next post we will share the results of testing the system, summarize and tell you about the difficulties encountered.

If you had a similar experience in the implementation of code style - it will be interesting to discuss this in the comments!