PL/SQL analysis with SonarQube – Majors

PLSQL_MajorsWe continue this series on the analysis of PL/ SQL code with today, the Major rules.

We saw previously how to organize our environment and configure our code analysis with Jenkins and SonarQube.

We created our own Quality Profile and reviewed the Blockers and the Criticals, all oriented toward Reliability and Security.

The Major rules

Looking at the Majors, we begin to have rules that affect Maintainability.

Majors10

I will move to Minors some good practices concerning the Readability or the Portability, because I do not want to make it a priority. And I will upgrade to Criticals some Majors concerning robustness, performance or security.

Why? Because these defects will impact the user. A bug that ends the application or leaves it in an unstable state, with a possible corruption of data, or with too long processing times will affect the user while defects on Readability or Understandibility will increase the charges and the costs of maintenance, but without direct consequences for the user. I want to put this kind of good practices into the background – in Majors or Minors issues.

This is of course a personal decision in the context of this analysis, designed to achieve a ‘demo’ environment that would allow me to show to a customer how to use SonarQube and for what benefits. I will not ignore defects that affect maintenance costs, but I want them to stay in the background and highlight violations of best practices that pose a risk to the end user.

Also, most of the code used to perform this analysis is quite old and from a time when these best programming practices were not very well established. The developers who currently maintain this application are not responsible for these defects, even though they weigh heavily on the technical debt.

Technical debt and SQALE

If we go into the SQALE page, the following pyramid shows that the robustness (Reliabililty – 163 days), performance (Efficiency – 192.5 days) and security (Security – 40 days) represent together 395.6 days of technical debt (costs of remediation of defects), or 14.7% of a total of 2 677.2 days.

PLSQL_Sqale1

The following diagram shows the importance taken by the Maintainability, which represents three quarters of the technical debt. We will talk more in a future post about this diagram SQALE Sunburst. But we can see (second circle) that Readability and Understandability are the two constitutive factors of maintenability. All the rules about Maintainability (third outermost circle) fall within these 2 groups.

PLSQL_SqaleSunburst1

The arrow shows that, in terms of Changeability, we should need 165.6 days of work to assign an alias to each column of all the tables, in order to facilitate the Understandibility and future developments (Changeability). This is more of a man-year, and I do not think that it is really the most critical.

In fact, there should be a specific project of more than 15 man-years to remove all the defects allocated to Maintainability, even though we do not know if this application will evolve or even exist much longer.

I’m going to move a number of good practices concerning Readability or Portability to Minors, because I do not want to make it a priority.

And move some Major rules concerning robustness, performance or safety to Criticals.
Without going into too much details, the rules that we upgrade to Criticals are:

  • EXCEPTION – Catch all exceptions with WHEN OTHERS
  • GOTO – Avoid use of GOTO statements
  • IF – Avoid nested if statements – Note: this one is related to Maintainability, but is critical enough to warrant this promotion. Especially that it can also impact, to some extent, the application Reliability, since it improves the risk of errors of logic.
  • LOOP – Avoid simple loops of the form LOOP … END LOOP
  • LOOP – Avoid using EXIT from within a FOR or WHILE loop
  • RETURN – Avoid using RETURN from within a loop
  • SQL – Avoid nested subqueries (queries in the WHERE clause)

I will even raise to Blockers the following rules:

  • SQL – Avoid using the GROUP BY clause
  • SQL – Avoid using UNION (celle-ci est de type Info).
  • SQL – Do not join on more than X tables
  • SQL – SELECT * should not be used

as they impact significantly the performance. They are also SQL rules that apply to all technologies (Cobol, ABAP, etc..). In fact, these rules do not present any defect for this application, so are known by developers.

It will not be the case for other languages, especially for ABAP projects: the knowledge of SQL best practices is not a strong point of SAP project teams.

The Major rules that we move to Minors:

  • FORMAT – Do not use more that one statement per line
  • LITERALS – Avoid using magic numbers
  • LITERAL – Avoid new-line or control characters in string literals
  • SQL – Columns should be aliased
  • SQL – Introduce column aliases using the AS keyword
  • SQL – Prefer EXECUTE IMMEDIATE to DBMS_SQL’s package procedure calls
  • SQL – Tables should be aliased
  • SQL – Use standard ANSI syntax instead of old syntax for join queries

Note: I change this repartition on the basis of the code analyzed. I usually do this work for each new customer, for its own rules (when he has). The plugin SQALE of SonarQube is a great help to do this.

Following this assignment, in our own Quality Profile, the allocation of the rules according to their criticity now seems more balanced:

Sqale_Rules

The number of ‘Blockers’ has not changed: the rules that we have moved into this category are clearly known and respected by the project team. However, there is always a possibility that some lack of attention or a new member in the team who does not know such rules will create such defects. In this case, our Quality Profile will be able to detect them.

The part of the technical debt depending on Criticals has increased, but in relatively limited way: it must be possible to correct these defects during the lifecycle of the project, or with a specific project that does not require a too heavy investment.

The number of Majors has declined since some rules were upgraded to Criticals or were downgraded to Minors. Most of these rules are oriented towards Maintainability, and a customer may wish to take into account all or part of this technical debt, according to its own IT orientation:

  • Avoid bugs for the users of this application.
  • Reduce maintenance costs, depending on the lifetime of the application, changes to be done, frequency of releases, budgets, etc.

The SQALE plugin provides a valuable help when adapting a Quality Profile according to the customer’s needs. It should be understood that the Quality Profile offered by default with SonarQube is a basis to customize the set of rules for each technology, and not a corpus of rules written in the marble. It is the role of the Quality consultant to adapt it according to the IT orientation of each customer, and to his specific needs, so that you can then build a realistic action plan, which allows to manage optimally the technical debt.

We’ll probably have the opportunity to work in a future post – and probably even more than one – on this SQALE plugin.

This post is also available in Leer este articulo en castellano and Lire cet article en français.