Code review en C#: qué revisar y cómo dar retroalimentación en la práctica

Code review en C#: qué revisar y cómo dar retroalimentación en la práctica

El code review no es solo un procedimiento formal antes de fusionar un pull request. Es una herramienta poderosa para mejorar la calidad del código, difundir conocimientos en el equipo y prevenir errores. Esto es especialmente relevante para C# y el ecosistema .NET, donde hay muchos matices: desde la gestión de memoria y la asincronía hasta la tipificación estricta y LINQ.

En este artículo, analizaremos en qué aspectos concretos fijarse al revisar código C# y cómo dar retroalimentación para que te escuchen, no te odien.

1. Arquitectura y lógica: "¿Y por qué hacemos esto?"

Antes de fijarte en el formato y los nombres de las variables, asegúrate de que el problema que se resuelve tenga sentido. Esta es la etapa más importante, pero a menudo se omite.

\n
• Cumplimiento de requisitos: ¿El código resuelve exactamente la tarea de negocio descrita en el ticket? ¿Hay funcionalidad innecesaria (YAGNI)?
\n
• Violación de principios SOLID: ¿La clase se ha convertido en un "objeto divino"? ¿Hay una clara separación de responsabilidades? Por ejemplo, ¿no se mezcla la lógica de negocio con la lógica de acceso a datos?
\n
• Complejidad excesiva: ¿Se puede resolver el problema de manera más simple? A veces los desarrolladores escriben patrones complejos donde basta con un simple if-else o un bucle simple.
\n

// ❌ Mal: La clase hace demasiado\npublic class UserService\n{\n    public void RegisterUser(string email, string password)\n    {\n        // Validación\n        if (!email.Contains("@"))\n            throw new Exception("Invalid email");\n\n        // Hash de contraseña\n        var hash = BCrypt.HashPassword(password);\n\n        // Guardar en BD\n        using var db = new AppDbContext();\n        db.Users.Add(new User { Email = email, PasswordHash = hash });\n        db.SaveChanges();\n\n        // Enviar email\n        SmtpClient.Send("welcome@site.com", email, "Welcome!");\n    }\n}\n\n// ✅ Bien: Separación en capas (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("Invalid email");\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. Rendimiento y seguridad: "¿Esto matará al servidor?"

.NET es un framework potente, pero es fácil usarlo incorrectamente. Esto es lo que debes revisar en cada code review.

\n
• Asincronía (async/await): ¿Hay llamadas bloqueantes (.Result, .Wait()) en código asíncrono? Esto mata el rendimiento. Usa ConfigureAwait(false) en bibliotecas.
\n
• Fugas de memoria: ¿Se olvidaron de liberar recursos IDisposable? ¿Se usa using para Stream, SqlConnection, etc.?
\n
• Inyección SQL: ¿Se construyen consultas mediante concatenación de cadenas? En C# moderno con Entity Framework o Dapper es raro, pero la verificación es obligatoria.
\n
• LINQ y colecciones: ¿Hay múltiples recorridos sobre la misma colección? ¿Se usa Any() en lugar de Count() > 0 para verificar la existencia de elementos?
\n

// ❌ Mal: Llamada bloqueante y recorrido múltiple\nvar task = GetDataAsync();\nvar data = task.Result; // ¡Bloqueo del hilo!\n\nif (data.Count() > 0) // Primer recor