Feedback

[rcellas]

Hola Jordi,
te dejo por aquí el feedback del proyecto. Cualquier duda que tengas lo podemos comentar.

1. Patrones de Código Limpio

Puntos Positivos

• Uso adecuado de inject(): Todos los servicios utilizan la función inject() moderna de Angular en lugar de inyección por constructor.
• Separación de responsabilidades: Los servicios están bien separados (StarshipsService, UserService).
• Uso de signals: Implementación correcta de signals para estado reactivo (isLogged, starshipList, nextPage).

Puntos de Mejora

• En starships-list.ts, el IntersectionObserver no se desconecta en ngOnDestroy, lo que podría causar fugas de memoria.
• Los componentes implementan OnInit y AfterViewInit pero no las declaran explícitamente con implements.
• En starship-detail.ts, la suscripción al Observable no se gestiona con takeUntil o async pipe, lo que puede causar fugas de memoria.
• Algunos servicios y propiedades no especifican modificadores (public/private) de manera consistente.
• Duplicación en formularios: Los componentes login.ts y register.ts tienen lógica de validación muy similar que podría abstraerse.
• Manejo de errores básico: Los bloques catch solo hacen console.error, falta un sistema de notificaciones al usuario más robusto.
• Magic numbers: En los tests hay valores hardcodeados como '123456' que deberían ser constantes.
• Poner firebase dentro de package.json en la parte de devDependencies para no tener problemas a la hora de desplegar la app en local.

---

2. HTML Semántico

Puntos Positivos

• Uso de <fieldset> en formularios: Los formularios utilizan correctamente <fieldset> y <legend>.
• Elementos de navegación: Uso correcto de <nav> con role="navigation" en el header.
• Listas semánticas: Uso apropiado de <ul> y <li> para el listado de naves.

Puntos de Mejora

• En starship-info.html se utiliza <article> conteniendo solo información de detalles, debería usar <section> o <dl> para listas de definiciones.
• La jerarquía de encabezados no es consistente (se salta de h2 a h3 sin contexto semántico claro).
• Las imágenes de fondo podrían tener atributos aria-hidden=true o role="presentation" si son decorativas.
• Se utiliza <main> como selector de componente, lo cual es correcto, pero podría mejorarse la estructura interna con más elementos semánticos como <section>.

---

3. Accesibilidad

Puntos Positivos

• ARIA labels: Excelente uso de aria-label, aria-labelledby, aria-describedby en formularios y navegación.
• ARIA states: Uso correcto de aria-invalid, aria-required, aria-disabled en inputs de formularios.
• Role attributes: Implementación apropiada de role="alert", role="status", role="navigation".
• Mensajes de error accesibles: Los mensajes de error están correctamente asociados con sus campos mediante aria-describedby.

Puntos de Mejora

• Variables CSS como --color-grey-light sobre --color-grey-hard podrían no cumplir con WCAG AA en términos de contraste de colores.

---

4. Buenas Prácticas de Estilos

Puntos Positivos

• Sistema de variables CSS: Excelente organización de variables CSS con categorías claras (colores, tipografía, espaciado).
• Nomenclatura BEM: Uso consistente de nomenclatura tipo BEM en las clases CSS.
• Mobile-first presente: Los media queries utilizan min-width indicando diseño mobile-first.
• Reseteo CSS: Implementación de reset básico con box-sizing: border-box.
• Uso de :host: Correcto uso del selector :host en componentes standalone.

Puntos de Mejora

• Algunas propiedades hardcodean valores que ya existen como variables CSS.
• Uso de selectores de clase anidados que podrían simplificarse.
• Uso extensivo de rem sin fallback o consideración de escalado extremo.

---

5. Code Smells

Puntos Positivos

• Métodos pequeños y enfocados: La mayoría de métodos tienen una responsabilidad única.
• Uso de pipeable operators: RxJS se utiliza con operadores modernos (switchMap, map, forkJoin).

Puntos de Mejora

• El método getStarShip() encadena múltiples operaciones RxJS que podrían dividirse en métodos auxiliares.
• Los componentes de auth tienen lógica de inicialización de formularios en el constructor; debería estar en ngOnInit.
• En UserService, los métodos register y login aceptan parámetros de tipo any.

---

6. Enrutamiento Optimizado en Angular

Puntos Positivos

• Protección de rutas: Uso correcto de guards de Firebase para proteger rutas autenticadas.
• Ruta wildcard: Implementación de ruta ** para manejar rutas no encontradas.
• Rutas parametrizadas: Uso correcto de parámetros dinámicos en rutas.

Puntos de Mejora

• Falta implementar lazy loading.
• No hay estrategia de precarga configurada.
• Los guards están implementados inline con canActivate(), podrían ser guards independientes para reutilización.
• La redirección de usuarios no autorizados en /starships redirige a /register, pero en /starships/:starshipName redirige a la home (inconsistencia).

---

7. Lógica en Services

Puntos Positivos

• Servicios singleton: Uso correcto de providedIn: 'root'.
• Uso de signals para estado: Implementación moderna usando signals en lugar de BehaviorSubject.
• Composición de observables: Uso avanzado de operadores RxJS para componer flujos de datos complejos.
• Inyección moderna: Uso de la función inject() en lugar de inyección en constructor.

Puntos de Mejora

• StarshipsService maneja tanto la obtención de datos como la transformación y paginación. Podría beneficiarse de un patrón repository.
• El uso de signals en el service es correcto, pero no hay mecanismo de reset o limpieza del estado.
• Los errores solo se loguean, sin estrategia de retry o manejo robusto.

---

8. Uso Adecuado de Signals

Puntos Positivos

• Signals para estado local: Uso correcto de signals para estado reactivo (isLogged, starshipList, nextPage).
• Update con función: Uso correcto de update() para modificar arrays de forma inmutable.
• WritableSignal tipado: Los signals están correctamente tipados.

Puntos de Mejora

• No se aprovechan computed() signals para valores derivados.

---

9. Patrones de Arquitectura de Componentes

Puntos Positivos

• Componentes standalone: Uso correcto de componentes standalone de Angular 20.
• Smart vs Presentational: Clara separación entre componentes contenedor (starship-detail) y presentacionales (starship-info, starship-films, etc.).
• Componentes pequeños y enfocados: Los componentes tienen responsabilidades bien definidas.

Puntos de Mejora

• Los componentes como starship-info, starship-films y starship-pilots son presentacionales pero podrían optimizarse con ChangeDetectionStrategy.OnPush.
• Los input() no están marcados como readonly en TypeScript.
• Los componentes son muy acoplados al router, podrían emitir eventos para mayor reutilización.

---

10. Cobertura de Tests

Puntos Positivos

• Tests unitarios básicos presentes: Todos los componentes tienen al menos un test básico.
• Uso de TestBed: Configuración correcta del entorno de testing.
• Mocking de dependencias: Uso apropiado de mocks para servicios externos.
• Tests de formularios reactivos: Hay tests específicos para validación de formularios en login y register.

Puntos de Mejora

• La mayoría de tests solo verifican creación de componentes con should create.
• Componentes como StarshipDetailComponent, StarshipInfoComponent, StarshipFilmsComponent, StarshipPilotsComponent solo tienen test de creación.
• Los tests de StarshipsService y UserService tienen cobertura básica pero no cubren todos los casos de error.
• En los tests de servicios, algunos mocks son demasiado simples y no reflejan comportamiento real.
• Aunque hay tests para validaciones, faltan tests para casos límite.
• No hay tests que verifiquen la interacción entre componentes y servicios.