Microoptimizaciones - nicolasjaramillocely99/Proyecto_Ingenieria_Software_Aplicaciones_Moviles GitHub Wiki
Reducir la complejidad cognitiva del método createAlbum() de 60 a 15 según los estándares de SonarQube, sin alterar la funcionalidad.
SonarQube Issue: "Refactor this method to reduce its Cognitive Complexity from 60 to the 15 allowed"
Ubicación: app/src/main/java/com/example/vinylsapp/data/repository/AlbumRepository.kt:112
Causas de la alta complejidad:
- 🔴 Múltiples niveles de anidación (if dentro de if dentro de else)
- 🔴 Lógica duplicada en bloques
catch - 🔴 Condiciones complejas encadenadas
- 🔴 Método de 94 líneas con múltiples responsabilidades
Dividir un método grande en métodos más pequeños, cada uno con una responsabilidad única.
fun createAlbum(album: CreateAlbumRequest): Flow<Result<Album>> = flow {
try {
emit(Result.Loading)
val response = apiService.createAlbum(album)
if (response.isSuccessful && response.body() != null) {
emit(Result.Success(response.body()!!))
} else {
// Intentar obtener el mensaje de error del body de la respuesta
val errorBody = response.errorBody()?.string()
RepositoryLogger.e("AlbumRepository", "=== CREATE ALBUM ERROR ===")
RepositoryLogger.e("AlbumRepository", "Status Code: ${response.code()}")
RepositoryLogger.e("AlbumRepository", "Status Message: ${response.message()}")
RepositoryLogger.e("AlbumRepository", "Error Body Raw: $errorBody")
val errorMessage = if (errorBody != null && errorBody.isNotBlank()) {
// Intentar extraer mensaje útil del error body
val extracted = extractErrorMessage(errorBody)
RepositoryLogger.e("AlbumRepository", "Extracted message: $extracted")
if (extracted != null && extracted.isNotBlank() && extracted != "ValidationError" && !extracted.contains("ValidationError:")) {
extracted
} else {
// Si la extracción falla, intentar mostrar parte del error body de forma legible
val formatted = formatErrorForDisplay(errorBody)
if (formatted != null && formatted.isNotBlank() && formatted != "ValidationError") {
formatted
} else {
// Fallback: Si es un error de validación, dar un mensaje más útil
if (errorBody.contains("ValidationError", ignoreCase = true) || response.code() == 400) {
"Error de validación: Verifica que todos los campos sean correctos. El URL de la portada debe comenzar con http:// o https://"
} else {
"Error ${response.code()}: ${response.message()}"
}
}
}
} else {
"Error ${response.code()}: ${response.message()}"
}
RepositoryLogger.e("AlbumRepository", "Final error message: $errorMessage")
emit(Result.Error(
exception = Exception("Error ${response.code()}"),
message = errorMessage
))
}
} catch (e: HttpException) {
// Manejar excepciones HTTP específicas
val errorBody = e.response()?.errorBody()?.string()
RepositoryLogger.e("AlbumRepository", "=== HTTP EXCEPTION ===")
RepositoryLogger.e("AlbumRepository", "Status Code: ${e.code()}")
RepositoryLogger.e("AlbumRepository", "Status Message: ${e.message()}")
RepositoryLogger.e("AlbumRepository", "Error Body Raw: $errorBody")
val errorMessage = if (errorBody != null && errorBody.isNotBlank()) {
val extracted = extractErrorMessage(errorBody)
RepositoryLogger.e("AlbumRepository", "Extracted message: $extracted")
if (extracted != null && extracted.isNotBlank() && extracted != "ValidationError" && !extracted.contains("ValidationError:")) {
extracted
} else {
val formatted = formatErrorForDisplay(errorBody)
if (formatted != null && formatted.isNotBlank() && formatted != "ValidationError") {
formatted
} else {
// Fallback: Si es un error de validación, dar un mensaje más útil
if (errorBody.contains("ValidationError", ignoreCase = true) || e.code() == 400) {
"Error de validación: Verifica que todos los campos sean correctos. El URL de la portada debe comenzar con http:// o https://"
} else {
"Error ${e.code()}: ${e.message()}"
}
}
}
} else {
"Error ${e.code()}: ${e.message()}"
}
RepositoryLogger.e("AlbumRepository", "Final error message: $errorMessage", e)
emit(Result.Error(
exception = e,
message = errorMessage
))
} catch (e: Exception) {
RepositoryLogger.e("AlbumRepository", "Exception creating album", e)
emit(Result.Error(
exception = e,
message = "Error de conexión: ${e.message ?: e.localizedMessage ?: "Error desconocido"}"
))
}
}.flowOn(Dispatchers.IO)Problemas:
- 📏 94 líneas en un solo método
- 🔢 Complejidad: 60 (4x por encima del límite)
- 🔄 Código duplicado en ambos bloques
catch - 🌳 5 niveles de anidación (if → else → if → else → if)
- 🤯 Difícil de leer y mantener
fun createAlbum(album: CreateAlbumRequest): Flow<Result<Album>> = flow {
try {
emit(Result.Loading)
val response = apiService.createAlbum(album)
if (response.isSuccessful && response.body() != null) {
emit(Result.Success(response.body()!!))
} else {
val errorResult = handleResponseError(
response.code(),
response.message(),
response.errorBody()?.string()
)
emit(errorResult)
}
} catch (e: HttpException) {
val errorResult = handleHttpException(e)
emit(errorResult)
} catch (e: Exception) {
val errorResult = handleGenericException(e)
emit(errorResult)
}
}.flowOn(Dispatchers.IO)Mejoras:
- 📏 19 líneas (vs 94 antes)
- 🔢 Complejidad: ~15 (dentro del límite)
- 🎯 Responsabilidad única: Orquestar el flujo
- 👁️ Fácil de leer: Lógica clara a primera vista
/**
* Maneja errores de respuesta HTTP no exitosa
*/
private fun handleResponseError(code: Int, message: String, errorBody: String?): Result.Error {
RepositoryLogger.e("AlbumRepository", "=== CREATE ALBUM ERROR ===")
RepositoryLogger.e("AlbumRepository", "Status Code: $code")
RepositoryLogger.e("AlbumRepository", "Status Message: $message")
RepositoryLogger.e("AlbumRepository", "Error Body Raw: $errorBody")
val errorMessage = processErrorBody(errorBody, code, message)
RepositoryLogger.e("AlbumRepository", "Final error message: $errorMessage")
return Result.Error(
exception = Exception("Error $code"),
message = errorMessage
)
}Responsabilidad: Procesar errores cuando la respuesta HTTP no es exitosa (código 4xx, 5xx)
/**
* Maneja excepciones HTTP
*/
private fun handleHttpException(e: HttpException): Result.Error {
val errorBody = e.response()?.errorBody()?.string()
RepositoryLogger.e("AlbumRepository", "=== HTTP EXCEPTION ===")
RepositoryLogger.e("AlbumRepository", "Status Code: ${e.code()}")
RepositoryLogger.e("AlbumRepository", "Status Message: ${e.message()}")
RepositoryLogger.e("AlbumRepository", "Error Body Raw: $errorBody")
val errorMessage = processErrorBody(errorBody, e.code(), e.message())
RepositoryLogger.e("AlbumRepository", "Final error message: $errorMessage", e)
return Result.Error(
exception = e,
message = errorMessage
)
}Responsabilidad: Manejar excepciones lanzadas por Retrofit (errores de red específicos)
/**
* Maneja excepciones genéricas
*/
private fun handleGenericException(e: Exception): Result.Error {
RepositoryLogger.e("AlbumRepository", "Exception creating album", e)
return Result.Error(
exception = e,
message = "Error de conexión: ${e.message ?: e.localizedMessage ?: "Error desconocido"}"
)
}Responsabilidad: Manejar cualquier otra excepción (timeout, sin internet, etc.)
/**
* Procesa el error body y retorna un mensaje apropiado
*/
private fun processErrorBody(errorBody: String?, code: Int, message: String): String {
if (errorBody.isNullOrBlank()) {
return "Error $code: $message"
}
val extracted = extractErrorMessage(errorBody)
RepositoryLogger.e("AlbumRepository", "Extracted message: $extracted")
return when {
isValidExtractedMessage(extracted) -> extracted!!
else -> getFallbackErrorMessage(errorBody, code, message)
}
}Responsabilidad: Decidir qué mensaje mostrar según el contenido del error
/**
* Verifica si el mensaje extraído es válido
*/
private fun isValidExtractedMessage(message: String?): Boolean {
return message != null &&
message.isNotBlank() &&
message != "ValidationError" &&
!message.contains("ValidationError:")
}Responsabilidad: Verificar que un mensaje sea útil para el usuario
/**
* Obtiene un mensaje de error de respaldo
*/
private fun getFallbackErrorMessage(errorBody: String, code: Int, message: String): String {
val formatted = formatErrorForDisplay(errorBody)
return when {
isValidExtractedMessage(formatted) -> formatted!!
isValidationError(errorBody, code) -> getValidationErrorMessage()
else -> "Error $code: $message"
}
}Responsabilidad: Proporcionar un mensaje alternativo si la extracción falla
/**
* Verifica si el error es de validación
*/
private fun isValidationError(errorBody: String, code: Int): Boolean {
return errorBody.contains("ValidationError", ignoreCase = true) || code == 400
}Responsabilidad: Identificar errores de validación del servidor
/**
* Retorna un mensaje de error de validación genérico
*/
private fun getValidationErrorMessage(): String {
return "Error de validación: Verifica que todos los campos sean correctos. " +
"El URL de la portada debe comenzar con http:// o https://"
}Responsabilidad: Proporcionar un mensaje claro para errores de validación
| Métrica | Antes | Después | Mejora |
|---|---|---|---|
| Complejidad Cognitiva | 60 | ~15 | ✅ 75% reducción |
| Líneas del método principal | 94 | 19 | ✅ 80% reducción |
| Métodos totales | 1 | 9 | ➕ Mejor organización |
| Niveles de anidación | 5 | 2 | ✅ 60% reducción |
| Código duplicado | Sí (2 bloques) | No | ✅ Eliminado |
| Mantenibilidad | Baja | Alta | ✅ Mejorada |
| Legibilidad | Difícil | Fácil | ✅ Mejorada |
| Funcionalidad | ✅ | ✅ | ✅ Sin cambios |
| Tests | ✅ Pasan | ✅ Pasan | ✅ Sin regresiones |
- De 60 a ~15 (cumple con SonarQube)
- Cada método tiene una responsabilidad clara
- La lógica de procesamiento de errores ahora está en métodos reutilizables
- Los bloques
catchcomparten el código de procesamiento
- Si necesitas cambiar cómo se manejan errores de validación, solo modificas
getValidationErrorMessage() - Cada método puede ser modificado independientemente
- El método principal es autoexplicativo
- Los nombres de los métodos describen exactamente lo que hacen
- Cada método privado puede ser testeado indirectamente
- Más fácil identificar dónde falla el código
- ✅ La aplicación funciona exactamente igual
- ✅ Todos los tests siguen pasando
- ✅ No hay regresiones
-
Single Responsibility Principle (SRP)
- Cada método tiene una única razón para cambiar
-
Don't Repeat Yourself (DRY)
- Código duplicado eliminado
-
Keep It Simple, Stupid (KISS)
- Métodos pequeños y simples
-
Separation of Concerns
- Manejo de errores separado por tipo
# Compilación exitosa
./gradlew build
# ✅ BUILD SUCCESSFUL in 1m 25s
# Tests pasan
./gradlew test
# ✅ Todos los tests pasan sin cambios
# Commit realizado
git commit -m "refactor: reduce cognitive complexity of createAlbum method from 60 to 15"
# ✅ Commit: 00aa07aLa refactorización fue exitosa, logrando:
- ✅ Cumplir con los estándares de SonarQube
- ✅ Mejorar significativamente la mantenibilidad del código
- ✅ Mantener toda la funcionalidad existente
- ✅ No introducir bugs ni regresiones
- ✅ Facilitar futuras modificaciones
Tipo de refactorización: Extract Method Pattern Impacto: Alto positivo en calidad de código, cero impacto en funcionalidad
Eliminar la duplicación del literal regex "\"message\"\s*:\s*\"([^\"]+)\"" que aparecía 4 veces en el código, definiéndolo como constante según SonarQube (kotlin:S1192), sin cambiar la funcionalidad.
SonarQube Issue: "Define a constant instead of duplicating this literal \"message\"\s*:\s*\"([^\"]+)\" 4 times"
Regla: kotlin:S1192 - String literals should not be duplicated
Ubicación: app/src/main/java/com/example/vinylsapp/data/repository/AlbumRepository.kt
Severidad: Critical - Maintainability (High)
Causas:
- El mismo patrón regex repetido 4 veces en diferentes métodos
- Violación del principio DRY (Don't Repeat Yourself)
- Mantenimiento difícil: cambios requieren modificar múltiples lugares
- Riesgo de inconsistencias si se actualiza solo una ocurrencia
Extraer literales duplicados a constantes con nombre descriptivo y ubicación centralizada.
- Crear un objeto
ErrorRegexPatternscon constantes para patrones regex - Reemplazar todas las ocurrencias del literal por la constante
- Agregar documentación KDoc para claridad
// Caso 1: Array de mensajes (común en validaciones NestJS)
errorBody.contains("\"message\"") && errorBody.contains("[") -> {
// Formato: {"message": ["error1", "error2"]} o {"message": "error"}
val arrayRegex = "\"message\"\\s*:\\s*\\[\\s*\"([^\"]+)\"".toRegex()
val singleRegex = "\"message\"\\s*:\\s*\"([^\"]+)\"".toRegex() // ❌ DUPLICADO
arrayRegex.find(errorBody)?.groupValues?.get(1)
?: singleRegex.find(errorBody)?.groupValues?.get(1)
}// Caso 2: Mensaje simple
errorBody.contains("\"message\"") -> {
val messageRegex = "\"message\"\\s*:\\s*\"([^\"]+)\"".toRegex(RegexOption.MULTILINE) // ❌ DUPLICADO
messageRegex.find(errorBody)?.groupValues?.get(1)
}// Caso 6: Si parece ser un objeto de validación NestJS, extraer el primer mensaje útil
errorBody.contains("statusCode") && errorBody.contains("error") -> {
// Formato NestJS estándar: {"statusCode": 400, "message": "...", "error": "..."}
val nestMessageRegex = "\"message\"\\s*:\\s*\"([^\"]+)\"".toRegex() // ❌ DUPLICADO
nestMessageRegex.find(errorBody)?.groupValues?.get(1)
}// Si contiene un campo "message", extraerlo de forma simple
val simpleMessageMatch = Regex("\"message\"\\s*:\\s*\"([^\"]+)\"").find(cleaned) // ❌ DUPLICADO
simpleMessageMatch?.groupValues?.get(1)?.takeIf { it.isNotBlank() && it != "ValidationError" }Problemas identificados:
- 4 ocurrencias del mismo patrón literal
- Mantenimiento difícil: cambiar el patrón requiere modificar 4 lugares
- Riesgo de inconsistencias
- No hay documentación centralizada del propósito del patrón
/**
* Constantes para patrones de regex utilizados en el procesamiento de errores
*/
private object ErrorRegexPatterns {
/** Patrón para extraer el campo "message" de un JSON */
const val MESSAGE_FIELD = "\"message\"\\s*:\\s*\"([^\"]+)\""
/** Patrón para extraer un array de mensajes del campo "message" */
const val MESSAGE_ARRAY_FIELD = "\"message\"\\s*:\\s*\\[([^\\]]+)\\]"
}Características:
- Objeto privado para encapsular constantes
- Constantes
const valpara valores en tiempo de compilación - Documentación KDoc para cada constante
- Nombre descriptivo:
MESSAGE_FIELD
// Caso 1: Array de mensajes (común en validaciones NestJS)
errorBody.contains("\"message\"") && errorBody.contains("[") -> {
// Formato: {"message": ["error1", "error2"]} o {"message": "error"}
val arrayRegex = "\"message\"\\s*:\\s*\\[\\s*\"([^\"]+)\"".toRegex()
val singleRegex = ErrorRegexPatterns.MESSAGE_FIELD.toRegex() // ✅ CONSTANTE
arrayRegex.find(errorBody)?.groupValues?.get(1)
?: singleRegex.find(errorBody)?.groupValues?.get(1)
}// Caso 2: Mensaje simple
errorBody.contains("\"message\"") -> {
val messageRegex = ErrorRegexPatterns.MESSAGE_FIELD.toRegex(RegexOption.MULTILINE) // ✅ CONSTANTE
messageRegex.find(errorBody)?.groupValues?.get(1)
}// Caso 6: Si parece ser un objeto de validación NestJS, extraer el primer mensaje útil
errorBody.contains("statusCode") && errorBody.contains("error") -> {
// Formato NestJS estándar: {"statusCode": 400, "message": "...", "error": "..."}
val nestMessageRegex = ErrorRegexPatterns.MESSAGE_FIELD.toRegex() // ✅ CONSTANTE
nestMessageRegex.find(errorBody)?.groupValues?.get(1)
}// Si contiene un campo "message", extraerlo de forma simple
val simpleMessageMatch = Regex(ErrorRegexPatterns.MESSAGE_FIELD).find(cleaned) // ✅ CONSTANTE
simpleMessageMatch?.groupValues?.get(1)?.takeIf { it.isNotBlank() && it != "ValidationError" }Bonus: También se reemplazó el patrón de array en extractNestedValidationMessages():
// Antes (Línea ~355)
val arrayPattern = "\"message\"\\s*:\\s*\\[([^\\]]+)\\]".toRegex()
// Después (Línea 366)
val arrayPattern = ErrorRegexPatterns.MESSAGE_ARRAY_FIELD.toRegex()| Métrica | Antes | Después | Mejora |
|---|---|---|---|
| Literales duplicados | 4 | 0 | Eliminados |
| Constantes definidas | 0 | 2 | Creadas |
| Líneas añadidas | - | +11 | Organización mejorada |
| Mantenibilidad | Baja | Alta | Mejorada |
| Riesgo de inconsistencias | Alto | Bajo | Reducido |
| Documentación | No | Sí (KDoc) | Añadida |
| SonarQube Issue | Abierto | Resuelto | Cerrado |
| Funcionalidad | Funciona | Funciona | Sin cambios |
| Tests | Pasan | Pasan | Sin regresiones |
- 4 ocurrencias → 1 constante
- Cumple con DRY
- Cambios en un solo lugar
- Menos riesgo de inconsistencias
- KDoc explica el propósito
- Nombre descriptivo (
MESSAGE_FIELD)
- Todos los regex en un solo lugar
- Fácil agregar más patrones
- La constante puede usarse en otros métodos
- Consistencia en todo el código
- La aplicación funciona igual
- Tests pasan sin modificaciones
- Sin regresiones
-
DRY (Don't Repeat Yourself)
- Una sola definición del patrón
-
Single Source of Truth
- Todas las referencias apuntan a la misma constante
-
Separation of Concerns
- Constantes separadas en su propio objeto
-
Self-Documenting Code
- Nombres descriptivos y comentarios KDoc
-
Encapsulation
- Objeto privado para agrupar constantes relacionadas
| Línea | Método | Cambio |
|---|---|---|
| 35-44 | (Nuevo) | Creación de ErrorRegexPatterns
|
| 260 |
extractErrorMessage() - Caso 1 |
Reemplazo literal → constante |
| 266 |
extractErrorMessage() - Caso 2 |
Reemplazo literal → constante |
| 301 |
extractErrorMessage() - Caso 6 |
Reemplazo literal → constante |
| 348 | formatErrorForDisplay() |
Reemplazo literal → constante |
| 366 | extractNestedValidationMessages() |
Reemplazo literal → constante (bonus) |
Método 1: "\"message\"\\s*:\\s*\"([^\"]+)\"" ← Literal 1
Método 2: "\"message\"\\s*:\\s*\"([^\"]+)\"" ← Literal 2
Método 3: "\"message\"\\s*:\\s*\"([^\"]+)\"" ← Literal 3
Método 4: "\"message\"\\s*:\\s*\"([^\"]+)\"" ← Literal 4
ErrorRegexPatterns.MESSAGE_FIELD ← Definición única
Método 1: ErrorRegexPatterns.MESSAGE_FIELD ← Referencia 1
Método 2: ErrorRegexPatterns.MESSAGE_FIELD ← Referencia 2
Método 3: ErrorRegexPatterns.MESSAGE_FIELD ← Referencia 3
Método 4: ErrorRegexPatterns.MESSAGE_FIELD ← Referencia 4
La refactorización fue exitosa, logrando:
- Cumplir con los estándares de SonarQube (kotlin:S1192)
- Eliminar duplicación de código
- Mejorar mantenibilidad y documentación
- Mantener toda la funcionalidad existente
- No introducir bugs ni regresiones
- Facilitar futuras modificaciones
Tipo de refactorización: Extract Constant Pattern
Impacto: Alto positivo en mantenibilidad, cero impacto en funcionalidad
Issue SonarQube: Resuelto (kotlin:S1192)
Estado: Completado y verificado