Article original : A list of bad practices commonly seen in industrial projects | Belay the C++ (belaycpp.com)
Traductrice : Chloé Lourseyre

Sur beaucoup de projets C++ industriel, il est très courant de voir un nombre conséquents de développeurs partager une même codebase.

Aujourd’hui, une grande majorité des développeurs C++ ne sont pas des experts du langage. Il y en a beaucoup qui sont issus d’autres langages avec d’autres pratiques, des gens qui ont seulement appris le C++ à l’école et qui ne sont pas spécialement intéressés par les mécaniques spécifiques au langage et qui ont des spécialités de plus haut niveau, ou encore des développeurs issus des temps anciens qui codent encore en « C avec des classes ».

L’idée de cet article est de lister une série (non-exhaustive) de mauvaises pratiques plutôt répandues afin que chacun, que que soit son niveau, puisse contribuer à une codebase plus saine et plus maintenable.

Mauvaise pratique : les fonctions trop longues

Je ne suis pas homme (ni femme) à imposer des limites strictes pour la longueur des fonctions. Cependant, quand on commence à avoir des fonctions qui font plusieurs milliers, voire plusieurs dizaines de milliers de lignes de code, il est nécessaire de mettre le holà.

Une fonction (comme tout bloc) est une unité architecturale du code. Plus elle est longue, plus elle est difficile à assimiler dans son entièreté. Si elle est divisée en unités plus petites, chaque unité est individuellement plus aisée à comprendre car l’esprit est tour-à-tour focalisé sur des problèmes de plus petite taille. Il suffit ensuite pour lui de les assembler pour comprendre la fonction dans sa globalité.

En d’autre terme, pour comprendre une grosse problématique, il est plus simple de la diviser en problématiques plus petites.

Ainsi, vos fonctions serviront parfois juste à appeler des fonctions auxiliaires en succession, ce qui est pratique car cela permet d’avoir le cheminement fonctionnel de la fonction sans avoir à se plonger dans le détail des implémentations.

La limite que je me fixe personnellement tourne autour de 200 lignes par fonction (parfois plus, parfois moins).

Mauvaise pratique : créer des classes là où il n’y en a pas besoin.

C’est quelque chose qui est surprenamment assez commun, de ce que j’ai pu en voir, probablement dû à des pratiques importées d’autres langages où la notion de classes est perçue différemment.

Il y a deux manières sous lesquelles cette mauvaise pratique peut survenir :

Les classes complètement statiques (avec parfois des constructeurs)

Il est plus simple d’illustrer cela avec un exemple, alors allons-y :

class MyMath
{
public:
    MyMath();
    ~MyMath();
    static int square(int i);
};
 
MyMath::MyMath()
{
}
 
MyMath::~MyMath()
{
}
 
int MyMath::square(int i)
{
    return i*i;
}
 
int main()
{
    MyMath mm;
    int j = mm.square(3);
    return j;
}

Voici les points problématiques avec ce code :

• Pourquoi implémenterait-on un constructeur et un destructeur vide, alors que ceux par défaut feraient très bien l’affaire ?
• Pourquoi implémenterait-on un constructeur et un destructeur dans une classe entièrement statique ?
• Pourquoi créerait-in une instance de cette classe juste pour appeler la méthode statique ?
• Pourquoi créerait-on une classe pour faire cela alors qu’un namespace ferait complètement l’affaire ?

Voici à quoi ce code devrait plutôt ressembler :

namespace MyMath
{
    int square(int i);
};
 
int MyMath::square(int i)
{
    return i*i;
}
 
int main()
{
    int j = MyMath::square(3);
    return j;
}

Plus concis, plus propre, meilleur sous tout rapport.

Certes, parfois les classes complètement statiques peuvent être utiles, mais dans les situations similaires à cette exemple, elles ne le sont pas.

Il n’y a aucun intérêt à utiliser une classe alors qu’on pourrait ne pas le faire. Si vous pensez que le namespace pourrait avoir besoin d’être transformé en classe dans le futur (avec des attributs et des méthodes), souvenez vous juste de ce petit dicton :

Ne codez jamais en prévision d’un futur hypothétique qui pourrait ou ne pourrait pas se réaliser. Le temps que vous passez à être trop prévoyant pour le futur est du temps perdu. Vous pourrez toujours refactorer plus tard au besoin.

Les classes complètement transparentes

J’ai mis ce sujet en second car il est beaucoup plus sujet à controverse que le premier.

Juste pour être clair : la seule différence entre une class et une struct, c’est que les membres d’une class sont privés par défaut, alors que les membres d’une struct sont publiques par défaut. C’est la seule différence.

Du coup, si votre classe :

• … n’a que des méthode publiques
• … a des accesseurs (get/set) pour tous ses attributs
• … n’a que des accesseur simplistes

… alors ce n’est pas une classe, c’est une structure.

Voici un exemple d’illustration :

class MyClass
{
    int m_foo;
    int m_bar;
 
public:
    int addAll();
    int getFoo() const;
    void setFoo(int foo);
    int getBar() const;
    void setBar(int bar);
};
 
int MyClass::addAll()
{ 
    return m_foo + m_bar;
}
int MyClass::getFoo() const
{
    return m_foo;
}
void MyClass::setFoo(int foo)
{
    m_foo = foo;
}
int MyClass::getBar() const
{
    return m_bar;
}
void MyClass::setBar(int bar)
{
    m_bar = bar;
}

Ce code peut être écrit beaucoup plus simplement avec une structure :

struct MyClass
{
    int foo;
    int bar;
 
    int addAll();
};
 
int MyClass::addAll()
{ 
    return foo + bar;
}

Ces deux codes sont assez équivalent, la seule réelle différence étant que la classe fournit un degré d’encapsulation supplémentaires (et inutile).

La controverse vient du caractère « inutile » que je viens de mentionner. En effet, dans une mentalité complètement orienté-objet, aucune encapsulation n’est inutile. De mon humble avis, ce genre de structure ne nécessite pas d’encapsulation car elle représente un ensemble de données stockées telles quelles, et je pense qu’il est contre-productif d’essayer de trop encapsuler le code de manière générale.

Attention cependant, car la pratique que je décrit ici n’est valide que si tout les attributs sont en accès direct à la fois en lecture et en écriture. Si vous avez des accesseurs spécifiques ou des attributs en lecture-seule ou écriture-seule, n’utilisez pas une struct (enfin, vous pouvez mais il faudra que vous y réfléchissiez sérieusement).

Mauvaise pratique : implémenter un comportement indéfini

Les undefined behavior, aussi appelés UB ou encore « comportements indéfinis » sont, pour faire court, un contrat que le développeur passe avec le compilateur, promettant à celui-ci qu’il n’implémentera pas certains comportements. Cela permet au compilateur de faire des hypothèses en prenant cela en compte et de faire des optimisations en conséquence.

Allez voir la conférence de Piotr Padlewski si vous voulez plus de détails à ce propos : CppCon 2017: Piotr Padlewski “Undefined Behaviour is awesome!” – YouTube.

Voici une liste non-exhaustive des UB que vous devez absolument connaître pour éviter d’avoir des comportements indéfinis à votre insu dans votre codebase :

• Appeler la fonction main()
• Le dépassement d’entier
• Le dépassement de buffer
• Utiliser des variables non-initialisées
• Déréférencer nullptr
• Omettre l’instruction return
• Nommer une variable en commençant avec deux underscores __
• Définir une fonction dans le namespace std
• Spécialiser un type qui n’a pas été défini par l’utilisateur dans le namespace std
• Prendre l’adresse d’une fonction de la std

En conséquence, et que ce soit dit une bonne fois pour toute : n’utilisez jamais, au grand jamais le dépassement d’entier comme condition de sortie d’une boucle ou pour toute autre raison. Parce que cela ne fonctionne pas comme vous le pensez et qu’un jour ça va se retourner contre vous.

Mauvaise pratique : comparer un entier signé avec un entier non-signé

Quand vous comparez un entier signé avec un non-signé, il y aura forcement une conversion arithmétique des types qui a une chance non-négligeable de fausser les valeurs et rendre caduque la comparaison.

Utilisez le type size_t quand c’est pertinent, et appliquez une static_cast sur vos données quand c’est nécessaire.

Mauvaise pratique : essayer d’optimiser le code tout en l’écrivant

Oui, c’est peut-être dur à avaler, mais voici deux faits :

• 80% du temps (voire même plus), le code que vous écrivez n’a pas besoin d’être optimisé. Puisque la plupart du temps d’exécution su programme survient dans 20% du code (principe de Pareto à l’œuvre), les 80% restants n’ont pas besoin d’être optimisés.
• L’optimisation ne doit pas être une préoccupation a priori. Ce que vous devez faire est : écrire votre code, prendre du recul et optimiser en conséquence.

La chose la plus importante à faire n’est pas d’optimiser votre programme, mais de vous arranger avant tout pour que votre code soit concis, élégant et maintenable. Si il l’est, vous pourrez toujours revenir plus tard pour optimiser si vous en avez effectivement besoin.

Mauvaise pratique : ne pas assez réfléchir

Certes, vous ne devez pas spécialement chercher à optimiser en écrivant du code, mais vous ne devez pas non plus tomber dans l’extrême inverse, c’est à dire sous-optimiser votre code à mesure que vous l’écrivez.

Connaissez les spécificités des algorithmes que vous utilisez, soyez conscients des buts des différentes structures de données et à quels besoins elles répondent, connaissez vos design pattern et soyez capables de les implémentez, et n’hésitez surtout pas à lire la doc des features que vous comptez utiliser.

Il y a un bon équilibre à atteindre entre coder sans réfléchir et la sur-optimisation.

Mauvaise pratique : « il faut faire ça parce qu’on en aura besoin plus tard… »

Je l’ai déjà dit et je le répète : ne codez pas en fonction d’un futur hypothétique, car tout peut changer au cours du développement. La seule chose qui est réellement inamovible, c’est le principe fondamental de votre programme, et rien de plus.

Vos besoins peuvent changer, vos specs peuvent changer, ce que le client demande peut changer, vos procédures peuvent changer, tout ce qui n’est pas le principe de base de votre programme peut changer. Parfois ça ne changera pas, mais souvent ces notions vont evoluer.

Si vous êtes en proie au doute, demandez-vous simplement :

Si j’implémente ça plus tard, est-ce que ça sera plus coûteux ?

Souvent, la réponse sera « non » ou « pas tant que ça ». Dans ce cas, il vaut mieux laisser le futur au futur.

En conclusion…

Là, vous avez un bon point de départ pour écrire du code élégant et surtout maintenable. Si tous les développeurs de votre projet appliquent ces règles, cela facilitera la vie de tout le monde.

Cependant il y a beaucoup d’autres mauvaises pratiques qui existent et ne sont pas listées ici. Peut-être publierai-je un autre article parlant de cela, peut-être pas.

Merci de votre attention et à la semaine prochaine !

Article original : A list of bad practices commonly seen in industrial projects | Belay the C++ (belaycpp.com)
Traductrice : Chloé Lourseyre