emrevarol.com

Materiales de la Clase

Mejores Prácticas de Codificación y Revisión Efectiva de Código

A2SV - University of Rwanda

Ver Presentación

Lista de Verificación de Revisión de Código A2SV

El estándar de oro. Diez categorías para evaluar cada pull request que revises.

1 Claridad y Legibilidad
Pregunta
  • ¿Los nombres de funciones y variables son claros?
  • ¿Puedo entender el código sin adivinar?
  • ¿La intención es obvia?
  • ¿La lógica es fácil de seguir?
Señales de Alerta
Nombres como x, tmp, data, calc, process; lógica profundamente anidada; funciones grandes y confusas
2 Calidad de Funciones y Diseño
Pregunta
  • ¿Cada función hace una sola cosa clara?
  • ¿La lógica de negocio está separada de los efectos secundarios?
  • ¿El diseño es fácil de probar?
  • ¿Alguna función está haciendo demasiado?
Señales de Alerta
Validación + guardado + correo + registro en una sola función; prints dentro de funciones lógicas; comportamiento codificado distribuido por todo el archivo
3 Corrección
Pregunta
  • ¿El código realmente resuelve el problema correctamente?
  • ¿Las suposiciones son explícitas?
  • ¿Se manejan todos los casos?
Señales de Alerta
Falta de validación de entrada; suposiciones sobre claves de diccionario; valores mágicos; fallo silencioso
4 Casos Límite
Pregunta
  • ¿Qué pasa con entrada vacía?
  • ¿Qué pasa con entrada inválida?
  • ¿Qué pasa con claves faltantes?
  • ¿Qué pasa con valores cero o negativos?
Señales de Alerta
Solo se considera el camino feliz; sin verificaciones defensivas donde se necesitan
5 Pruebas
Pregunta
  • ¿Se incluyen pruebas?
  • ¿Las pruebas verifican resultados exactos?
  • ¿Se cubren los casos límite?
  • ¿Estas pruebas detectarían regresiones?
Señales de Alerta
Solo una prueba; aserciones débiles como > 0; sin casos de entrada inválida; sin pruebas para la lógica modificada
6 Duplicación
Pregunta
  • ¿Se repite la lógica?
  • ¿Se puede extraer la lógica compartida?
  • ¿Las reglas de negocio están duplicadas en múltiples lugares?
Señales de Alerta
Fórmulas repetidas; bloques de condiciones repetidos; valores codificados repetidos
7 Manejo de Errores
Pregunta
  • ¿Los errores se manejan de forma clara?
  • ¿El código falla de manera comprensible?
  • ¿Los estados inválidos son visibles?
Señales de Alerta
Retornar None sin explicación; excepciones tragadas; modos de fallo ocultos
8 Mantenibilidad
Pregunta
  • ¿Será fácil de extender más adelante?
  • ¿Otro ingeniero podrá modificar esto de forma segura?
  • ¿Esto hace que el código base sea más limpio o más desordenado?
Señales de Alerta
Reglas codificadas; PR excesivamente grande; lógica fuertemente acoplada
9 Calidad del Repositorio
Pregunta
  • ¿La estructura del repositorio es clara?
  • ¿El README es útil?
  • ¿Un nuevo ingeniero puede ejecutar el proyecto fácilmente?
  • ¿Las dependencias son visibles?
Señales de Alerta
Sin instrucciones de configuración; README débil; estructura confusa; directorio de pruebas faltante
10 Calidad del PR
Pregunta
  • ¿El PR está enfocado?
  • ¿El título es descriptivo?
  • ¿La descripción es útil?
  • ¿El tamaño del cambio es razonable?
Señales de Alerta
Títulos vagos; sin explicación; cambios no relacionados mezclados; PRs gigantes

Plantillas de Comentarios de Revisión

Frases profesionales para escenarios comunes de revisión. Úsalas como punto de partida.

Nomenclatura
"¿Podríamos renombrar esta función para reflejar mejor su propósito?"
"Este nombre de variable parece demasiado genérico. ¿Podemos hacerlo más descriptivo?"
Lógica
"¿Podríamos simplificar esta condición para mejorar la legibilidad?"
"¿Tendría sentido extraer esta lógica en una función auxiliar?"
Pruebas
"¿Podemos agregar una prueba para el caso de entrada vacía?"
"¿Podríamos verificar el valor exacto esperado aquí en lugar de usar una comprobación amplia?"
Estructura
"Esta función parece estar haciendo múltiples cosas. ¿Podemos separar las responsabilidades?"
"¿Dividir esto en funciones más pequeñas lo haría más fácil de probar y revisar?"
Casos Límite
"¿Qué pasa si falta esta clave?"
"¿Deberíamos validar la entrada antes de procesarla?"
Calidad del PR
"¿Podrías agregar una breve descripción del PR explicando el cambio y cómo debería probarse?"

Demostración en Vivo: Revisa Este Pull Request

Un PR deliberadamente defectuoso para practicar. ¿Puedes encontrar los problemas?

Rama: feature/cart-total-update
Título: "Update cart calculation logic"
Descripción: "Updated the cart calculation logic and added support for discount."
src/cart.py
def calc(items, user, d=False): t = 0 for i in items: if i["type"] == "book": t += i["price"] * 0.9 else: t += i["price"] if d: t = t - t * 0.1 if user is not None: if "is_premium" in user: if user["is_premium"] == True: t = t - t * 0.05 return t def process_cart(items, user): total = calc(items, user, True) print("cart processed") return total
tests/test_cart.py
from src.cart import calc def test_calc(): items = [{"type": "book", "price": 100}] user = {"is_premium": True} assert calc(items, user, True) > 0
README.md
# Cart Project cart logic
Guía de Revisión - Problemas a Encontrar
Problemas a Nivel de PR
  • Título de PR vago: "Update cart calculation logic" no describe qué cambió
  • La descripción es genérica e inútil
  • Sin plan de pruebas ni instrucciones para los revisores
Problemas a Nivel de Código
  • El nombre de función calc es demasiado genérico - debería ser calculate_cart_total
  • El parámetro d no es claro - debería ser discount_enabled
  • Las variables t e i no son descriptivas
  • Números mágicos: 0.9, 0.1, 0.05 deberían ser constantes con nombre
  • Bloques if profundamente anidados para la verificación premium - usar .get()
  • process_cart tiene un efecto secundario (print) mezclado con lógica de negocio
  • Sin validación de entrada para items o user
  • El parámetro booleano d=False pasado posicionalmente como True es confuso
Problemas a Nivel de Pruebas
  • Solo un caso de prueba para todo el módulo
  • Aserción débil: > 0 no verifica la corrección
  • Sin pruebas de casos límite (lista vacía, claves faltantes, usuario no premium)
  • No prueba process_cart
Problemas a Nivel de Repositorio
  • El README está esencialmente vacío - sin instrucciones de configuración, sin descripción
  • Sin información sobre cómo ejecutar las pruebas
Versión Mejorada - cart_improved.py
src/cart_improved.py
BOOK_DISCOUNT_RATE = 0.10 GENERAL_DISCOUNT_RATE = 0.10 PREMIUM_DISCOUNT_RATE = 0.05 def calculate_item_price(item): item_type = item["type"] price = item["price"] if item_type == "book": return price * (1 - BOOK_DISCOUNT_RATE) return price def apply_general_discount(total_price, discount_enabled): if not discount_enabled: return total_price return total_price * (1 - GENERAL_DISCOUNT_RATE) def apply_premium_discount(total_price, user): if not user or not user.get("is_premium", False): return total_price return total_price * (1 - PREMIUM_DISCOUNT_RATE) def calculate_cart_total(items, user, discount_enabled=False): total_price = 0 for item in items: total_price += calculate_item_price(item) total_price = apply_general_discount(total_price, discount_enabled) total_price = apply_premium_discount(total_price, user) return total_price def process_cart(items, user): total_price = calculate_cart_total(items, user, discount_enabled=True) return total_price

Tarea

Tarea de Revisión de PR: Revisa Como un Ingeniero Profesional

Tu entrega debe incluir:

  • 10 total Mínimo de comentarios de revisión
  • 3 mín Comentarios relacionados con nomenclatura
  • 2 mín Comentarios relacionados con pruebas
  • 2 mín Comentarios sobre diseño o responsabilidad de funciones
  • 1 mín Comentario sobre calidad del repositorio
  • 1 mín Sugerencia de refactorización

Formato de Entrega

File: src/orders.py
Line/Area: calc function
Comment: The function name is too generic. Could we rename
it to calculate_order_total for clarity?

File: tests/test_orders.py
Line/Area: test_calc
Comment: This test only checks that the result is greater
than zero. Could we assert the exact expected value instead?

Revisa los siguientes archivos:

src/orders.py
def calc(items, vip=False): total = 0 for i in items: if i["type"] == "book": total += i["price"] * 0.9 elif i["type"] == "electronics": total += i["price"] else: total += i["price"] if vip == True: total = total - total * 0.05 return total def checkout(items, user): total = calc(items, user["vip"]) print("checking out order...") print("total is", total) return {"ok": True, "total": total}
src/utils.py
def f(x): if x == None: return False return True
tests/test_orders.py
from src.orders import calc def test_calc(): items = [{"type": "book", "price": 100}] assert calc(items, False) > 0
README.md
# Orders Run it with python.
Los grandes ingenieros no solo hacen que el código funcione. Hacen que el código sea comprensible, verificable, revisable y confiable.
Frase del Día
"El código que hoy estás demasiado ocupado para limpiar se convierte en el problema que mañana domina a tu equipo."