Code review en JavaScript : quoi vérifier et comment donner un retour d'information

Code review en JavaScript : quoi vérifier et comment donner un retour

Code review (revue de code) n'est pas simplement une vérification formelle avant de merger une pull request. C'est un outil puissant pour améliorer la qualité du code, partager les connaissances au sein de l'équipe et prévenir les bugs dès les premières étapes. Dans le monde de JavaScript et Node.js, où l'asynchronisme et le typage dynamique créent de nombreux pièges cachés, une revue compétente devient cruciale.

Dans cet article, nous allons détailler sur quoi porter spécifiquement attention lors de la revue de code JavaScript, et comment donner un retour qui soit utile, sans être blessant.

1. Logique, architecture et sécurité

La première chose à vérifier est si le code résout le problème posé. Ne vous plongez pas immédiatement dans le formatage. Posez-vous ces questions :

• La logique fonctionne-t-elle correctement ? Y a-t-il des cas limites (edge cases) ? Que se passe-t-il si on reçoit null, undefined ou un tableau vide ?
• Y a-t-il des vulnérabilités ? En Node.js, c'est particulièrement pertinent. Vérifiez si des données brutes d'une requête ne sont pas passées directement dans une requête SQL (SQL injection) ou dans eval(). Des sanitizers sont-ils utilisés pour le HTML ?
• L'architecture est-elle respectée ? Le code ne viole-t-il pas les principes SOLID ? Par exemple, une seule fonction ne fait-elle pas trop de choses ? La logique métier est-elle séparée des gestionnaires de routes ?

// MAUVAIS : Mélange de la logique et de la couche HTTPapp.get('/user/:id', async (req, res) => {    const user = await db.query(`SELECT * FROM users WHERE id = ${req.params.id}`); // SQL Injection !    res.json(user);});

// BON : Séparation des responsabilitésconst getUserById = async (id) => {    if (!id || typeof id !== 'string') throw new Error('ID invalide');    return await db.query('SELECT * FROM users WHERE id = $1', [id]);};app.get('/user/:id', async (req, res) => {    const user = await getUserById(req.params.id);    res.json(user);});

2. Asynchronisme et travail avec les promesses

C'est le talon d'Achille de nombreux développeurs JavaScript. Lors de la revue de code Node.js, vérifiez impérativement :

• La gestion des erreurs. Toutes les fonctions asynchrones sont-elles enveloppées dans un try/catch ? Le .catch() des promesses n'est-il pas oublié ? Y a-t-il des promesses "orphelines" (unhandled promise rejections) ?
• L'utilisation correcte d'async/await. N'y a-t-il pas un await superflu là où les promesses pourraient être lancées en parallèle avec Promise.all ?
• Les conditions de course (race conditions). Le code pourrait-il s'exécuter incorrectement lors de requêtes parallèles vers une même ressource (par exemple, un fichier ou une variable globale) ?

// MAUVAIS : Exécution séquentielle de requêtes indépendantesconst user = await getUser(id);const posts = await getPosts(id);

// BON : Exécution parallèleconst [user, posts] = await Promise.all([getUser(id), getPosts(id)]);

// MAUVAIS : Gestion d'erreur oubliéerouter.get('/data', async (req, res) => {    const data = await fetchData(); // Si fetchData échoue, le serveur "tombe" aussi    res.json(data);});

// BONrouter.get('/data', async (req, res, next) => {    try {        const data = await fetchData();        res.json(data);    } catch (error) {        next(error); // On transmet l'erreur au gestionnaire global    }});

3. Qualité du code, style et nommage

Le code doit être lisible, comme un bon livre. Même si la logique est correcte, un mauvais style ralentira le développement à l'avenir.

• Nommage des variables et fonctions. Les noms doivent répondre aux questions "qu'est-ce que c'est ?" et "qu'est-ce que ça fait ?". Évitez les variables à une lettre (sauf pour les compteurs de boucle) et les abréviations comme getUsrInf.
• Nombres et chaînes magiques. Sont-ils extraits dans des constantes ? const MAX_RETRIES = 3; est bien plus compréhensible que