Revue de code en C# : que vérifier et comment donner un retour d'expérience en pratique

Revue de code en C# : que vérifier et comment donner un retour d'expérience en pratique

La revue de code n'est pas simplement une procédure formelle avant la fusion d'une pull request. C'est un outil puissant pour améliorer la qualité du code, diffuser les connaissances au sein de l'équipe et prévenir les bogues. Cela est particulièrement pertinent pour C# et l'écosystème .NET, où il existe de nombreuses nuances : de la gestion de la mémoire et de l'asynchronisme à la typage strict et à LINQ.

Dans cet article, nous allons détailler sur quoi se concentrer spécifiquement lors de la revue de code C# et comment donner un retour d'expérience pour que l'on vous écoute, sans vous détester.

1. Architecture et logique : "Pourquoi faisons-nous cela ?"

Avant de regarder le formatage et les noms de variables, assurez-vous que la tâche à résoudre a du sens. C'est l'étape la plus importante, mais souvent négligée.

\n
• Conformité aux exigences : Le code résout-il exactement le problème métier décrit dans le ticket ? N'y a-t-il pas de fonctionnalités superflues (YAGNI) ?
\n
• Violation des principes SOLID : La classe ne s'est-elle pas transformée en "objet divin" ? Y a-t-il une séparation claire des responsabilités ? Par exemple, la logique métier n'est-elle pas mélangée avec la logique d'accès aux données ?
\n
• Complexité excessive : Peut-on résoudre le problème plus simplement ? Parfois, les développeurs écrivent des motifs complexes là où un simple if-else ou une boucle simple suffit.
\n

// ❌ Mauvais : La classe fait trop de choses\npublic class UserService\n{\n    public void RegisterUser(string email, string password)\n    {\n        // Validation\n        if (!email.Contains("@"))\n            throw new Exception("Email invalide");\n\n        // Hachage du mot de passe\n        var hash = BCrypt.HashPassword(password);\n\n        // Sauvegarde en base de données\n        using var db = new AppDbContext();\n        db.Users.Add(new User { Email = email, PasswordHash = hash });\n        db.SaveChanges();\n\n        // Envoi d'email\n        SmtpClient.Send("welcome@site.com", email, "Bienvenue !");\n    }\n}\n\n// ✅ Bon : Séparation en couches (SRP)\npublic class UserRegistrationService\n{\n    private readonly IUserRepository _repository;\n    private readonly IEmailService _emailService;\n\n    public UserRegistrationService(IUserRepository repository, IEmailService emailService)\n    {\n        _repository = repository;\n        _emailService = emailService;\n    }\n\n    public async Task<Result> RegisterAsync(string email, string password)\n    {\n        if (!EmailValidator.IsValid(email))\n            return Result.Failure("Email invalide");\n\n        var user = User.Create(email, password);\n        await _repository.AddAsync(user);\n        await _emailService.SendWelcomeAsync(email);\n\n        return Result.Success();\n    }\n}

2. Performance et sécurité : "Cela va-t-il tuer le serveur ?"

.NET est un framework puissant, mais il est facile de l'utiliser incorrectement. Voici ce qu'il faut vérifier dans chaque revue.

\n
• Asynchronisme (async/await) : Y a-t-il des appels bloquants (.Result, .Wait()) dans du code asynchrone ? Cela tue les performances. Utilisez ConfigureAwait(false) dans les bibliothèques.
\n
• Fuites de mémoire : A-t-on oublié de libérer les ressources IDisposable ? Utilise-t-on using pour Stream, SqlConnection, etc. ?
\n
• Injections SQL : Les requêtes sont-elles construites par concaténation de chaînes ? Dans le C# moderne avec Entity Framework ou Dapper, c'est rare, mais la vérification est obligatoire.
\n
• LINQ et collections : Y a-t-il plusieurs parcours de la même collection ? Utilise-t-on Any() au lieu de Count() > 0 pour vérifier la présence d'éléments ?
\n

// ❌ Mauvais : Appel bloquant et parcours multiple\nvar task = GetDataAsync();\nvar data = task.Result; // Blocage du thread !\n\nif (data.Count() > 0) // Premier parc