Nous continuons cette série sur l’analyse de code PL/SQL, avec aujourd’hui les règles de type Major.

Nous avons vu précédemment comment organiser notre environnement et configurer notre analyse de code avec Jenkins et SonarQube.

Nous avons créé notre propre Quality Profile, puis examiné les règles de type Blockers et Criticals, toutes orientées Robustesse (Reliability) et Sécurité.

Les règles de type Major

Si on passe aux Majors, on commence à avoir des règles qui impactent la maintenabilité.

Je vais faire passer en Minors un certain nombre de bonnes pratiques qui concernent la lisibilité du code ou sa portabilité, car je ne souhaite pas en faire une priorité. Et faire passer en Criticals certaines règles Majors qui concernent la robustesse, la performance ou la sécurité.

Pour quelles raisons ? Parce que ces défauts vont impacter l’utilisateur. Un bug qui ‘sort’ l’utilisateur de l’application ou laisse celle-ci dans un état instable, avec une possible corruption de données, ou un temps de traitement trop long vont affecter l’utilisateur alors que des défauts portant sur la lisibilité ou la compréhension du code vont augmenter le travail – et donc les coûts et les temps – de maintenance, mais sans conséquences directes pour l’utilisateur. Je souhaite donc mettre ce type de bonnes pratiques au second plan – en issues de type Major voire Minor.

Il s’agit bien entendu d’une décision personnelle dans le cadre de cette analyse, destinée à réaliser une ‘démo’ qui puisse me permettre de démontrer à un client comment utiliser SonarQube et quels bénéfices en tirer. Je ne vais pas ignorer des défauts qui affectent les coûts de maintenance, mais je souhaite les faire passer au second plan et mettre en avant les violations aux bonnes pratiques qui présentent un risque pour l’utilisateur final.

Egalement, la plupart du code utilisé pour effectuer cette analyse est assez ancien, et date d’une époque où ces bonnes pratiques de programmation n’étaient pas en vigueur. Les développeurs qui doivent maintenir actuellement ce code ne sont pas responsables de ces défauts, alors même qu’ils pèsent lourdement sur la dette technique.

Technical dette et SQALE

Si nous allons dans la page Squale, la pyramide suivante nous montre que la Robustesse (Reliabililty – 163 jours), la performance (Efficiency – 192.5 jours) et la sécurité (Security – 40 jours) représentent ensemble 395.6 jours de dette technique (coûts de remédiation des défauts correspondants) sur un total de 2 677.2, soit 14.7%.

Le diagramme suivant nous montre bien l’importance prise par la maintenabilité, qui représente les 3/4 de la dette technique. Nous reviendrons dans un prochain post sur ce diagramme SQALE Sunburst. Mais nous pouvons voir (second cercle) que la lisibilité (Readibility) et la compréhension du code (Understandability) sont les 2 facteurs constitutifs de la Maintenability. Toutes les règles (troisième cercle, le plus extérieur) concernant la Maintenabilité se répartissent au sein de ces 2 groupes.

La flèche nous montre qu’en matière de Changeabilité, il faudrait 165.6 jours de travail pour affecter un alias à chaque colonne des tables afin d’en faciliter la compréhension et les évolutions futures (Changeability). Cela représente plus d’une année-homme, et je ne pense pas que cela soit vraiment le plus critique. Ou en tout cas pas autant qu’un défaut qui impacte l’utilisateur.

En fait, il faudrait un chantier spécifique de plus de 15 années hommes afin de supprimer la part de la dette technique imputable à la Maintenabilité, alors même que nous ignorons si cette application va évoluer, voire même exister encore longtemps.

Je vais faire donc faire passer en Minors un certain nombre de bonnes pratiques qui concernent la lisibilité du code ou sa portabilité, car je ne souhaite pas en faire une priorité. Et faire passer en Critical certaines règles Major qui concernent la robustesse, la performance ou la sécurité.

Sans rentrer dans trop de détails, les règles que nous passons en Criticals sont les suivantes :

• EXCEPTION – Catch all exceptions with WHEN OTHERS
• GOTO – Avoid use of GOTO statements
• IF – Avoid nested if statements – Remarque : celle-ci concerne la maintainabilité, mais me paraît suffisamment critique pour justifier cette promotion. D’autant qu’elle peut impacter également, dans une certaine mesure, la robustelle de l’application, puisqu’elle favorise le risque d’erreurs de logique applicative.
• 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)

Je vais passer en Blockers les règles suivantes :

• 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

car celles-ci impactent la performance de manière importante. Ce sont d’ailleurs des règles SQL qui s’appliquent à toutes les technologies (Cobol, ABAP, etc.). En fait, ces règles ne présentent pas de défauts, donc sont connues par les développeurs. Dans le cadre de cette analyse et de ce code.

Ce ne sera pas le cas pour d’autres langages, particulièrement en matière de développement ABAP : la connaissances des bonnes pratiques SQL n’est pas un point fort des équipes de projet SAP.

Les règles Major que nous passons en Minor :

• 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

Remarque : je modifie cette répartition sur la base du code analysé. J’ai l’habitude de réaliser ce travail pour chaque nouveau client, sur la base de ses propres règles. Le plugin SQALE de SonarQube est d’une grande aide pour ce faire.

A la suite de cette affectation, dans notre propre Quality Profile, la répartition des règles selon leur criticité me paraît maintenant plus équilibrée :

Le nombre de défaut de type ‘Blockers’ n’a pas évolué : les règles que nous avons passé dans cette catégorie sont manifestement connues et respectées par l’équipe de projet.

La dette technique imputable aux règles Criticals a par contre augmentée, mais de manière relativement limitée : il doit être possible de corriger ces défauts au cours du cycle de vie de projet, voire avec un chantier spécifique qui ne nécessite pas un investissement trop lourd.

Le nombre de Majors a baissé, puisque certaines règles sont passées en Criticals ou ont été degradées en Minor. La plupart de ces règles Major sont orientées Maintenabilité, et un client pourra décider de prendre en compte toute ou partie de cette dette technique, en fonction de sa propre orientation IT :

• Eviter les bugs pour les utilisateurs de cette application.
• Réduire les coûts de maintenance, en fonction de la durée de vie de l’application, des évolutions à effectuer, de son budget, etc.

Le plugin SQALE offre une aide précieuse pour adapter un Quality Profile en fonction des besoins du client. Il faut bien comprendre que le Quality Profile offert par défaut par SonarQube pour chaque technologie, est une base de travail et non pas un corpus de règles inscrites dans le marbre. C’est le rôle du consultant Qualité de qualifier l’orientation IT de chaque client, et d’adapter ce Quality Profile de manière spécifique à ses besoins, de manière à pouvoir construire ensuite un plan d’actions réaliste, qui permette de gérer la dette technique de manière optimale.

Nous aurons probablement l’occasion de consacrer dans le futur un – et même probablement plusieurs posts à ce plugin SQALE.

Cette publication est également disponible en Leer este articulo en castellano : liste des langues séparées par une virgule, Read that post in english : dernière langue.