165 lines
6.8 KiB
Markdown
165 lines
6.8 KiB
Markdown
|
|
# Location Module — Code Review
|
||
|
|
|
||
|
|
## Resumen Ejecutivo
|
||
|
|
|
||
|
|
~6000 líneas de Dart. Arquitectura limpia pero con **3 bugs críticos** y varias oportunidades de mejora.
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Bugs Críticos (HIGH)
|
||
|
|
|
||
|
|
### 1. Crash por array out of bounds en el playback del historial
|
||
|
|
**Archivo:** `location_map.dart:753`
|
||
|
|
```dart
|
||
|
|
currentPosition: widget.positionHistory[mapState.historyNavigationIndex]
|
||
|
|
```
|
||
|
|
Si el historial se limpia mientras el playback está activo, `historyNavigationIndex` puede apuntar fuera del array → crash.
|
||
|
|
|
||
|
|
### 2. El historial NO se limpia al cambiar de dispositivo
|
||
|
|
**Archivo:** `location_map.dart:126-133`
|
||
|
|
Cuando el usuario cambia de dispositivo, la ruta histórica del dispositivo anterior sigue visible en el mapa. Confusión garantizada.
|
||
|
|
|
||
|
|
### 3. Rebuilds excesivos de marcadores con historial grande
|
||
|
|
**Archivo:** `location_map.dart:796-882`
|
||
|
|
`_buildMarkers()` se llama en cada build. Con 1000+ posiciones de historial, recrea la lista completa de markers cada vez → UI laggy/janky.
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Bugs Medios (MEDIUM)
|
||
|
|
|
||
|
|
### 4. Race condition en animación de historial
|
||
|
|
**Archivo:** `location_map.dart:141-152`
|
||
|
|
Si se carga nuevo historial mientras la animación sigue, pueden haber conflictos de estado. No hay guard contra animaciones concurrentes.
|
||
|
|
|
||
|
|
### 5. Memory leak con timers
|
||
|
|
**Archivo:** `location_map.dart:99-109`
|
||
|
|
`_followTimer` se recrea en `didUpdateWidget` sin garantías de que el callback anterior terminó su ejecución.
|
||
|
|
|
||
|
|
### 6. Errores silenciosos en carga inicial
|
||
|
|
**Archivo:** `location_controller.dart:31-43`
|
||
|
|
Si geofences Y frequent places fallan al cargar, el catch-all devuelve estado vacío sin notificar al usuario.
|
||
|
|
|
||
|
|
### 7. Paginación de historial sin límite de memoria
|
||
|
|
**Archivo:** `location_remote_datasource_impl.dart:128-150`
|
||
|
|
`pageSize=1000` con loop hasta `totalPages`. Datasets enormes pueden causar OOM al acumular todas las posiciones en memoria.
|
||
|
|
|
||
|
|
### 8. Cast inseguro en markers
|
||
|
|
**Archivo:** `location_map.dart:812`
|
||
|
|
```dart
|
||
|
|
(historyLayer.build(context) as MarkerLayer).markers
|
||
|
|
```
|
||
|
|
Puede crashear si cambia el tipo de retorno de `RouteHistoryLayer.build()`.
|
||
|
|
|
||
|
|
### 9. Clustering O(n²)
|
||
|
|
**Archivo:** `route_history_layer.dart:160-182`
|
||
|
|
Algoritmo de clustering compara cada punto con todos los demás. Lento para datasets grandes.
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Bugs Menores (LOW)
|
||
|
|
|
||
|
|
### 10. Filtro de coordenadas (0,0)
|
||
|
|
**Archivo:** `location_remote_datasource_impl.dart:152-154`
|
||
|
|
`latitude != 0 || longitude != 0` podría filtrar posiciones válidas cerca del ecuador/meridiano de Greenwich.
|
||
|
|
|
||
|
|
### 11. Sin backoff en polling
|
||
|
|
**Archivo:** `location_map.dart:103-108`
|
||
|
|
Timer sigue disparando cada N segundos aunque el dispositivo esté desconectado. Solo hace `return` temprano, sin reducir frecuencia.
|
||
|
|
|
||
|
|
### 12. Tracking unawaited sin error handling
|
||
|
|
Las llamadas de analytics (`unawaited(tracking.event())`) no manejan errores. Difícil de debuggear si el tracking falla silenciosamente.
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Lo que funciona bien
|
||
|
|
|
||
|
|
- **CRUD completo** de geofences y frequent places con buen manejo de 404
|
||
|
|
- **Tests sólidos**: 577 + 312 líneas cubriendo todos los CRUD y transiciones de estado
|
||
|
|
- **Animaciones** del mapa (reveal, playback, move) bien implementadas
|
||
|
|
- **Separación de controllers**: LocationController (datos) vs LocationMapController (UI del mapa)
|
||
|
|
- **Error propagation** con enums tipados y keys i18n para cada tipo de error
|
||
|
|
- **Tracking analytics** exhaustivo en todas las acciones del usuario
|
||
|
|
- **Paginación** en historial de posiciones (loop hasta totalPages)
|
||
|
|
- **orderBy correcto** en historial (positionDate ASC) y posiciones actuales (positionDate DESC)
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Lo que falta
|
||
|
|
|
||
|
|
- **Sin WebSocket** para posiciones en tiempo real (solo HTTP polling cada 60s+)
|
||
|
|
- **Sin widget tests** ni integration tests
|
||
|
|
- **Sin tests de performance** con datasets grandes
|
||
|
|
- **Sin tests de concurrencia** (CRUD simultáneo, device switch durante playback)
|
||
|
|
- **LocationMap tiene 1063 líneas** — debería dividirse en subwidgets
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Arquitectura
|
||
|
|
|
||
|
|
### Data Flow
|
||
|
|
```
|
||
|
|
API (SaveFamilyRepository)
|
||
|
|
→ LocationRemoteDatasource (HTTP calls)
|
||
|
|
→ LocationRepository (error mapping, 404 handling)
|
||
|
|
→ LocationController (state management, business logic)
|
||
|
|
→ LocationScreen → LocationMap → Widgets
|
||
|
|
```
|
||
|
|
|
||
|
|
### State Management
|
||
|
|
- **LocationController** (keepAlive: true) — geofences, frequent places, position history, CRUD operations
|
||
|
|
- **LocationMapController** — UI state: placing mode, selections, animations, playback, reveal
|
||
|
|
- **LocationListFilterController** — simple enum filter for position history type
|
||
|
|
|
||
|
|
### Map Layers (bottom to top)
|
||
|
|
1. TileLayer (estilo del mapa)
|
||
|
|
2. CircleLayer (geofences activos)
|
||
|
|
3. PolylineLayer (ruta del historial)
|
||
|
|
4. CircleLayer (preview de geofence durante creación)
|
||
|
|
5. MarkerLayer (dispositivo, geofences, frequent places, posiciones del historial)
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Fixes recomendados por prioridad
|
||
|
|
|
||
|
|
### Prioridad 1 — Prevenir crashes
|
||
|
|
1. **Bounds check en playback** — Verificar `historyNavigationIndex < positionHistory.length` antes de acceder
|
||
|
|
2. **Limpiar historial al cambiar dispositivo** — Llamar `clearPositionHistory()` en device switch
|
||
|
|
|
||
|
|
### Prioridad 2 — Mejorar rendimiento
|
||
|
|
3. **Memoizar markers** — Cache del resultado de `_buildMarkers()`, solo recalcular cuando cambian los datos
|
||
|
|
4. **Limitar posiciones en memoria** — Cap de posiciones o streaming en lugar de acumular todo
|
||
|
|
|
||
|
|
### Prioridad 3 — Mejorar robustez
|
||
|
|
5. **Guard de animación concurrente** — Flag `_isAnimating` para prevenir inicio de nueva animación mientras otra corre
|
||
|
|
6. **Mostrar error si carga inicial falla** — Propagar error al usuario en lugar de silenciar
|
||
|
|
|
||
|
|
### Prioridad 4 — Mejorar UX
|
||
|
|
7. **WebSocket para posiciones** — Reemplazar polling HTTP por push via WebSocket
|
||
|
|
8. **Dividir LocationMap** — Extraer subwidgets para controles, markers, info cards
|
||
|
|
|
||
|
|
---
|
||
|
|
|
||
|
|
## Archivos del módulo
|
||
|
|
|
||
|
|
### Core (Data Layer)
|
||
|
|
- `src/core/data/datasource/location_remote_datasource.dart` — interfaz abstracta
|
||
|
|
- `src/core/data/datasource/location_remote_datasource_impl.dart` — 157 líneas, HTTP calls
|
||
|
|
- `src/core/data/repositories/location_repository_impl.dart` — 102 líneas, error mapping
|
||
|
|
- `src/core/domain/entities/geofence_entity.dart` — Freezed entity
|
||
|
|
- `src/core/domain/entities/frequent_place_entity.dart` — Freezed entity
|
||
|
|
- `src/core/data/models/` — 14 DTOs con json_serializable
|
||
|
|
- `src/core/providers/` — 2 providers de DI
|
||
|
|
|
||
|
|
### Presentation (Feature Layer)
|
||
|
|
- `location_controller.dart` — 431 líneas, business logic principal
|
||
|
|
- `location_map_controller.dart` — 328 líneas, state machine del mapa
|
||
|
|
- `location_state.dart` — 66 líneas, Freezed state
|
||
|
|
- `location_map_state.dart` — 37 líneas, Freezed UI state
|
||
|
|
- `location_screen.dart` — 82 líneas, entry point
|
||
|
|
- `location_map.dart` — **1063 líneas**, widget principal del mapa
|
||
|
|
- `widgets/` — 8 controles de mapa + 4 info cards + 4 widgets de soporte
|
||
|
|
|
||
|
|
### Tests
|
||
|
|
- `location_controller_test.dart` — 577 líneas, CRUD completo
|
||
|
|
- `location_map_controller_test.dart` — 312 líneas, transiciones de estado
|