No simplifiques este código

 (github.com/kubernetes)
2 puntos por GN⁺ 2024-08-07 | 1 comentarios | Compartir por WhatsApp
  • pv_controller.go de Kubernetes es un controlador que sincroniza el binding PV/PVC y, desde la parte superior del archivo, deja claro que “no se debe simplificar” y que hay que mantener el space shuttle style.
  • Este estilo consiste en incluir un else correspondiente para cada if y dejar comentarios incluso para condiciones que parecen obvias, con el fin de hacer visibles en el código las ramas revisadas y su intención.
  • El centro del diseño son los punteros bidireccionales entre pvc.Spec.VolumeName y pv.Spec.ClaimRef, que permiten manejar de forma recuperable carreras, eliminaciones, modificaciones de usuarios y bindings simultáneos en un entorno sin transacciones.
  • El controlador combina observación de cambios en PV/PVC, caché interna, una cola de un solo worker, registro de eventos, aprovisionamiento dinámico e interfaces de migración CSI para gestionar las transiciones de estado del binding.
  • Las ramas y comentarios extensos son un mecanismo para preservar el conocimiento de negocio del comportamiento y el contexto de recuperación ante fallas, por lo que los cambios futuros deben seguir el mismo estilo.

Rol y principios de escritura de pv_controller.go

  • pv_controller.go es el archivo de implementación de PersistentVolumeController en el paquete persistentvolume de Kubernetes.
  • Este controlador alinea el estado de PersistentVolumeClaim y PersistentVolume.
    • Controlador de caché que observa cambios en PersistentVolume.
    • Controlador de caché que observa cambios en PersistentVolumeClaim.
    • Sincronización del estado de PV/PVC con base en los eventos de cambio de ambos objetos.
  • El comentario al inicio del archivo advierte repetidamente que este código no debe simplificarse.
    • El nombre del estilo es space shuttle style.
    • Consiste en tener un else correspondiente para cada sentencia if.
    • Su objetivo es explicitar todas las ramas, salvo las verificaciones simples de errores.
    • Incluso las acciones que parecen obvias se escriben como comentarios para que quienes mantengan el código puedan seguir la complejidad del binding.

Por qué mantener el space shuttle style

  • Este controlador es el resultado de combinar en uno solo el trabajo que originalmente estaba dividido en tres controladores.
  • En el proceso de simplificar el subsistema de PV, se volvió necesario manejar explícitamente todas las condiciones en el código.
  • Como resultado, el código puede parecer verboso y tener muchos comentarios y ramas.
  • Esa verbosidad es un mecanismo para dejar en el código el conocimiento de negocio y el contexto del comportamiento de binding.
  • Al modificar este archivo, se debe preservar el space shuttle style y, cuando sea necesario, agregar ramas y comentarios de la misma manera.

Diseño central: punteros bidireccionales entre PV y PVC

  • En el centro del diseño hay punteros bidireccionales entre PV y PVC.
    • Puntero del lado del PVC: pvc.Spec.VolumeName.
    • Puntero del lado del PV: pv.Spec.ClaimRef.
  • Esta bidireccionalidad es difícil de manejar en un sistema sin transacciones, pero es necesaria para garantizar un funcionamiento correcto incluso ante fallas.
  • Si una instancia rogue de controlador HA genera una condición de carrera, pueden aparecer múltiples bindings indistinguibles, lo que crea posibilidad de pérdida de datos.
  • El controlador está diseñado básicamente para operar en modo de alta disponibilidad active-passive.
    • Las transiciones de objetos están diseñadas para poder funcionar también con HA active-active.
    • Sin embargo, si dos controladores activos chocan con frecuencia, el rendimiento puede disminuir.

Formas de binding y condiciones de recuperación

  • El controlador admite objetos pre-bound bidireccionales.
    • Un PVC que quiere un PV específico.
    • Un PV reservado para un PVC específico.
  • El binding se realiza en dos etapas.
    • Primero se modifica PV.Spec.ClaimRef.
    • Luego se modifica PVC.Spec.VolumeName.
  • En cualquier momento de este proceso, el PV o el PVC puede ser modificado o eliminado por un usuario u otro controlador.
  • También es posible que dos o más controladores intenten vincular distintos volúmenes y claims al mismo tiempo.
  • El controlador debe poder recuperarse de este tipo de conflictos.

Componentes principales de la estructura del controlador

  • PersistentVolumeController tiene los listers, funciones de sincronización de informers, cliente de Kubernetes, registrador de eventos, gestor de plugins de volúmenes y otros componentes necesarios para sincronizar PV/PVC.
  • Las últimas versiones conocidas de PV/PVC se guardan en una caché interna.
    • volumes persistentVolumeOrderedIndex.
    • claims cache.Store.
  • Esta caché refleja tanto la versión más reciente guardada en el servidor API como la versión recibida mediante eventos de etcd.
  • Un binding puede generar aproximadamente cuatro eventos.
    • Actualización de volume.Spec.
    • Actualización de volume.Status.
    • Actualización de claim.Spec.
    • Actualización de claim.Status.
  • Sin una caché interna, cuando el informer conserva un estado antiguo, podría intentar volver a corregir un binding que ya fue completado.
  • En ese caso, si se intenta escribir de nuevo en el servidor API, puede producirse un conflicto de versiones con el objeto ya guardado.

Cola de trabajo y restricciones de concurrencia

  • El controlador tiene workqueues separadas para procesar claims y volúmenes.
    • claimQueue.
    • volumeQueue.
  • Cada cola debe tener exactamente un solo worker thread.
  • En particular, syncClaim() no es reentrante.
  • Si dos syncClaim() se ejecutan al mismo tiempo, pueden aparecer los siguientes problemas.
    • Vincular dos claims distintos al mismo volumen.
    • Vincular un claim a dos volúmenes.
  • El controlador puede recuperarse de estas situaciones mediante errores de versión del servidor API y sus propias verificaciones, pero un enfoque multi-worker puede reducir la velocidad total.

syncClaim: punto de entrada de la sincronización de PVC

  • syncClaim es el método principal que se invoca cuando se crea, actualiza o sincroniza periódicamente un claim.
  • Este método no distingue el tipo de evento.
  • Primero establece la annotation de migración correcta en el PVC y, si hace falta, lo actualiza en el servidor API.
  • Luego ramifica según la presencia de la annotation AnnBindCompleted.
    • Si no existe la annotation, syncUnboundClaim.
    • Si existe la annotation, syncBoundClaim.
  • Por legibilidad, el procesamiento real se divide en métodos para claims unbound y bound.

checkVolumeSatisfyClaim: verificación de requisitos del PV

  • checkVolumeSatisfyClaim verifica si el PV solicitado satisface los requisitos del PVC.
  • Las condiciones de verificación están enumeradas explícitamente en el código.
    • Error si el PV tiene DeletionTimestamp.
    • Error si la capacidad del PV es menor que la capacidad solicitada por el PVC.
    • Error si storageClassName es distinto.
    • Si el feature gate VolumeAttributesClass está activado, verifica que VolumeAttributesClassName coincida.
    • Si el feature gate está desactivado pero el claim o el volumen tiene VolumeAttributesClassName, devuelve error.
    • Error si volumeMode no es compatible.
    • Error si el access mode no es compatible.
  • Si todas las condiciones se cumplen, devuelve nil.

Manejo de eventos para PVC con binding diferido

  • emitEventForUnboundDelayBindingClaim crea un evento informativo para un claim sin binding en modo de binding diferido.
  • El reason predeterminado es WaitForFirstConsumer.
  • El mensaje predeterminado indica que el binding esperará hasta que se cree el primer consumer.
  • Si hay un Pod aún no programado que referencia ese PVC, el reason cambia a WaitForPodScheduled.
    • Si hay varios Pods, el mensaje incluye los nombres de todos ellos.
    • En volume scheduling solo se considera un Pod, pero como no se puede saber qué Pod se usará, se incluyen todos.

syncUnboundClaim: procesamiento de un PVC aún no vinculado

  • Si claim.Spec.VolumeName está vacío, significa que el usuario no solicitó un PV específico.
  • En este caso, el controlador revisa el modo de binding diferido del claim y busca el PV más adecuado con findBestMatchForClaim.
  • Si no hay un PV adecuado, procede en el siguiente orden.
    • Si puede asignar una StorageClass predeterminada, actualiza el PVC y termina la sincronización.
    • Si es binding diferido y aún no está en estado de provisioning, genera un evento de espera.
    • Si el claim tiene StorageClass, intenta aprovisionamiento dinámico con provisionClaim.
    • En caso contrario, registra un evento FailedBinding indicando que no hay PV disponible ni StorageClass.
  • Si hay un PV adecuado, llama a bind para vincular el PV y el PVC.
    • Si tiene éxito, registra la métrica del trabajo de provisioning + binding y limpia la caché de timestamps.
    • Si ocurre un error al guardar, un syncClaim posterior terminará el binding.

Procesamiento de un PVC que solicita un PV específico

  • Si claim.Spec.VolumeName no está vacío, significa que el usuario solicitó un PV específico.
  • Si el PV solicitado no está en la caché, actualiza el estado del PVC a Pending y reintenta más adelante.
  • Si el PV solicitado existe y volume.Spec.ClaimRef no existe, el PV aún no ha sido reclamado.
    • Verifica los requisitos con checkVolumeSatisfyClaim.
    • Si no satisface los requisitos, registra un evento VolumeMismatch y mantiene el PVC en Pending.
    • Si satisface los requisitos, llama a bind.
  • Si el PV solicitado ya fue reclamado por este PVC, llama a bind para completar el binding.
  • Si el PV solicitado está asociado a otro claim, lo procesa así:
    • Si el claim no tiene una annotation que indique que fue vinculado por el controlador, registra un evento FailedBinding y lo deja en Pending.
    • Si parece que fue vinculado por el controlador pero está asociado a otro claim, devuelve un error en un estado que “should never happen”.

syncBoundClaim: procesamiento de un PVC ya vinculado

  • syncBoundClaim procesa un PVC que tiene la annotation AnnBindCompleted.
  • Si el claim ya vinculado tiene claim.Spec.VolumeName vacío, cambia el estado del claim a ClaimLost.
    • El mensaje del evento indica que el claim bound perdió la referencia al PV y que los datos del volumen se perdieron.
  • Si el PV apuntado por el claim no existe, también lo cambia a ClaimLost.
    • El mensaje del evento indica que el claim bound perdió el PersistentVolume y que los datos se perdieron.
  • Si el PV existe pero volume.Spec.ClaimRef no existe, considera que el volumen volvió a estar unbound y llama de nuevo a bind.
  • Si ClaimRef.UID del PV es igual al UID del claim, considera que el binding es normal y llama a bind.
    • En la mayoría de los casos, esta llamada no hace nada.
  • Si el PV apunta a otro claimant, establece la phase del claim en el estado terminal Lost.

syncVolume: punto de entrada de la sincronización de PV

  • syncVolume es el método principal que se invoca al crear, actualizar o sincronizar periódicamente un volumen.
  • No distingue el tipo de evento.
  • Primero establece la annotation de migración y el finalizer correctos en el PV y, si hace falta, lo actualiza en el servidor API.
  • Si volume.Spec.ClaimRef no existe, considera que es un volumen no usado y establece la phase en Available.
  • Si hay ClaimRef pero el UID está vacío, considera que es un PV reservado para un PVC específico y establece la phase en Available.
    • Ese PVC aún no está vinculado a este PV, y el sync del PVC se encargará de procesarlo.

Procesamiento de un PV cuyo claim no se encontró

  • Si el PV está vinculado a un claim, el controlador busca el PVC usando namespace/name de ClaimRef.
  • Si no encuentra el PVC en la caché, realiza verificaciones adicionales bajo ciertas condiciones.
    • Vuelve a verificar en el informer cache.
    • Vuelve a verificar en el servidor API.
  • En los PV creados por un provisioner externo de PV o un binder externo de PV, bajo carga alta, el PVC podría no haberse sincronizado todavía en la caché local.
  • Para evitar reclamar incorrectamente el PVC, realiza una doble verificación.
  • Si determina que el claim no existe, cambia la phase del volumen a Released y ejecuta reclaimVolume.
    • Si la phase existente es Failed, no la sobrescribe.
    • Si la reclaim policy es Retain, deja un log indicando que el PV referencia un claim inexistente.

Cuando la conexión entre PV y PVC está desalineada

  • Si el claim existe pero claim.Spec.VolumeName está vacío, el PVC aún no tiene el nombre del PV.
  • Si volumeMode no coincide, registra un evento VolumeMismatch tanto en el PV como en el PVC y omite syncClaim.
  • Si no hay mismatch, agrega el claim a claimQueue para que syncClaim sea llamado pronto.
    • Este enfoque acelera el binding de volúmenes aprovisionados.
  • Si Spec.VolumeName del claim coincide con el nombre del volumen actual, lo considera un binding normal y actualiza la phase del volumen a Bound.
  • Si el claim está vinculado a otro volumen, lo procesa según la situación.
    • Si es un volumen aprovisionado dinámicamente y la reclaim policy es Delete, lo marca como Released y ejecuta reclaimVolume.
    • Si es un volumen vinculado por el controlador, lo limpia con unbindVolume.
    • Si es un puntero creado por el usuario, lo deja como está pero llama a unbindVolume para actualizar la phase y limpiar ClaimRef.UID.

Actualización de estado y emisión de eventos

  • updateClaimStatus guarda el status del PVC en el servidor API.
    • Cambio de phase.
    • Inicialización de AccessModes, Capacity, CurrentVolumeAttributesClassName cuando no hay volumen.
    • Actualización del access mode, capacity y nombre de current volume attributes class cuando hay volumen.
  • Hay una condición que actualiza la capacity solo en el momento en que el claim pasa a Bound.
    • La diferencia entre el tamaño del filesystem del PVC y el tamaño del block device del PV puede ser intencional, por lo que no sobrescribe la capacity de un claim que ya está bound.
  • Si el feature gate VolumeAttributesClass está activado, establece CurrentVolumeAttributesClassName durante la transición de pending a bound.
    • Después de eso, debe encargarse el resizer o un override de admin; si el controlador lo sigue configurando, puede haber una race condition.
  • updateClaimStatusWithEvent y updateVolumePhaseWithEvent emiten eventos solo cuando cambia realmente el status/phase.

Asignación de StorageClass predeterminada

  • assignDefaultStorageClass busca y asigna una StorageClass predeterminada cuando el claim no tiene storage class.
  • Ignora los claims que ya tienen storage class.
  • Si no existe una class predeterminada, no actualiza nada y devuelve false.
  • Si existe una class predeterminada, establece el nombre de la class en claim.Spec.StorageClassName y actualiza el servidor API.

Alcance del archivo y límites explícitos

  • Según los metadatos del archivo mostrados en la página de GitHub, pv_controller.go tiene 2038 líneas, 1864 LOC, 91 KB.
  • El texto proporcionado incluye solo desde la parte inicial del archivo hasta el comienzo de la función bindVolumeToClaim; el resto continúa en el enlace de raw view.
  • Por lo tanto, este resumen se limita a la estructura del controlador, los comentarios de diseño, las principales ramas de sincronización y la lógica de actualización de estado visibles en el cuerpo de código proporcionado.

Lecturas relacionadas

1 comentarios

 
GN⁺ 2024-08-07
Comentarios de Hacker News

  • No sé si es raro que el código de este archivo me parezca realmente código Go común y corriente. En Go suele ser verboso, y como no depende de abstracciones profundas parece más largo, pero el código en sí se ve típico.
    Las abstracciones son un arma de doble filo, así que este enfoque también me parece bien, y si no hubiera visto la introducción probablemente no habría pensado dos veces en el estilo en que está escrito. Quizá la diferencia venga de que tengo más experiencia con software empresarial que con software de sistemas. A alguien que contribuye constantemente a Kubernetes estos comentarios podrían parecerle innecesarios, pero si fuera código de un entorno empresarial que un lector muy en el futuro va a leer sin contexto, con esta complejidad yo más bien habría agregado todavía más comentarios

    • Antes este tipo de código me parecía normal, pero en los últimos 10 años más o menos siento que mucha gente ha empezado a valorar más la brevedad que la explicitud.
      Sobre todo en código importante como este, prefiero por mucho la explicitud. Varias veces en mi carrera me he topado con código que combina varias condiciones y omite comentarios que expliquen el contexto de negocio y el significado, y entonces no se puede saber si el comportamiento actual es intencional o accidental. Ese estilo tiende a producir código que no resiste bien los cambios, sino que los bloquea, y al menos hace que sea difícil modificarlo para cualquiera que no sea el autor. Crear vallas de Chesterton innecesarias va en contra de la mantenibilidad

    • Este comentario probablemente se agregó después de intentar simplificar el código y fracasar, como advertencia para que futuros mantenedores lo piensen dos veces antes de intentar lo mismo.
      El commit que agregó la advertencia fue "Add note about space-shuttle code style"[1], y el commit inmediatamente anterior fue "Revert controller/volume: simplify sync logic in syncUnboundClaim"[2]

      [1] https://github.com/kubernetes/kubernetes/commit/de4d193d45f6...

      [2] https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca...

    • Yo también pensaba algo parecido, pero cambié de opinión al ver los if muy anidados. En esa parte definitivamente habría creado ramas de retorno temprano.
      Da la impresión de que siguieron solo la primera etapa de "haz que funcione, hazlo rápido, hazlo bonito" y no hicieron la de "hacerlo bonito". Yo también he escrito código feo y lleno de comentarios al desenredar interacciones de estado complicadas, pero normalmente lo ordeno un poco antes del review. Tal vez simplemente sería mejor poner un gran letrero al principio del archivo que diga "no intentes simplificar este código". Aun así, definitivamente no está tan mal

    • Puede parecer raro, pero no eres el único. A mí este código me parece completamente normal. He escrito código y comentarios así en componentes que considero importantes para la confiabilidad del sistema.
      Nunca estuve de acuerdo con la moda del "código sin comentarios", y cuando vuelvo meses o años después, demasiadas veces me ha pasado que los comentarios que escribí fueron valiosísimos para mi yo del futuro. Me cuesta imaginar volver a reconstruir la lógica incrustada en un componente de esta complejidad sin comentarios sólidos

    • En particular, la explicación de que cada if tiene su comentario else correspondiente no parece ser consistentemente cierta. Muchos de los if sin par son simples chequeos if (err != nil) { u otros retornos tempranos, pero incluso excluyendo esos casos, sí parece haber if sin correspondencia.
      Dicho eso, por mi experiencia en software empresarial, tampoco diría que haya demasiados comentarios extra. En los codebases abundaban los comentarios // end if como una plaga, pero los comentarios realmente explicativos eran raros

  • Artículo sobre la calidad del software del Space Shuttle: https://archive.is/HX7n4
    Cito un fragmento: lo asombroso de este software no es cuántas cosas hace, sino qué tan bien funciona. Nunca se cae, no requiere reinicios, no tiene bugs y, a nivel de lo que ha logrado la humanidad, está cerca de la perfección. Las últimas tres versiones tenían 420 mil líneas cada una y solo un error por versión, y en las últimas 11 versiones completas hubo 17 errores. Un programa comercial de complejidad similar habría tenido alrededor de 5,000 errores

    • Me pregunto qué significa exactamente eso de que "las últimas tres versiones tenían 420 mil líneas y un error cada una". Si cada una de las tres versiones tuvo exactamente un bug, ¿no sería una forma rara de decir que las dos correcciones anteriores no funcionaron o que introdujeron bugs nuevos?
    • Sería interesante comparar en qué se diferencian el enfoque de la NASA y el enfoque de SpaceX. SpaceX también ha realizado misiones tripuladas, así que los requisitos parecen bastante parecidos
    • 5000 / 17 ≈ 295. ¿Sería justo asumir que un programa comercial de la misma complejidad requirió 295 veces menos horas-persona?
    • El problema de la metodología de desarrollo del Space Shuttle es que es absurdamente cara y lenta, y aun así no está 100% libre de bugs.
      Es tan cara y lenta que probablemente sería mucho más barato, más rápido y en la práctica más seguro demostrar la corrección del software con asistentes modernos de prueba formal (proof assistants). Proyectos como seL4 y CompCert muestran cómo debería hacerse
    • Es uno de mis artículos favoritos. Me sorprende que un texto de internet de 1996 siga siendo accesible

  • Entiendo la intención de // KEEP THE SPACE SHUTTLE FLYING., pero da un poco de risa que el comentario haga referencia a un sistema que ya no opera y que no tiene precisamente un gran historial de seguridad.
    ¿Dentro de unos 10 años la gente seguirá recordando al Space Shuttle de manera positiva?

    • Los problemas de seguridad del Space Shuttle fueron en gran parte problemas de hardware, no de software.
      En "Appendix F - Personal Observations on Reliability of Shuttle" [0], el apéndice de Richard Feynman incluido en el informe del accidente del Challenger de 1986, se dice lo siguiente

      En resumen, el sistema y la actitud de revisión del software de computadora son de la más alta calidad. No se ve el proceso de ir relajando los criterios y engañarse gradualmente a sí mismos, tan característico de los sistemas de seguridad del Solid Rocket Booster o del Space Shuttle Main Engine.

      Él destacó específicamente la calidad del software de aviónica como ejemplo de que incluso un gran proyecto gubernamental complejo como el Shuttle puede estar correctamente diseñado desde el punto de vista de la ingeniería, y que no está condenado por naturaleza a ser de baja calidad o peligroso

0: https://www.nasa.gov/history/rogersrep/v2appf.htm

  • Lanzó personas y equipo al espacio y los trajo de vuelta a casa en mucho más de 100 misiones exitosas. Incluso hoy se le sigue viendo favorablemente y probablemente siga siendo así. Fue un éxito en términos de progreso humano y efecto neto positivo

  • Lo que terminó con el Shuttle no fue un mal historial de seguridad, sino el costo y la expectativa de un deterioro futuro en la seguridad
    Aunque en los dos accidentes del Shuttle murieron más astronautas que en otros desastres de la NASA, considerando la dificultad real de lo que ocurrió, su historial de seguridad fue realmente asombroso. El código se ve bastante bien

  • La situación del Space Shuttle es más compleja que simplemente decir que era inseguro. Si se mide por misión, su historial es mejor que el de otros vehículos de lanzamiento. El Shuttle tuvo 2 misiones fatales de 135, mientras que el Soyuz de la era soviética tuvo 2 de 66, y SpaceShipTwo tiene un historial aterradoramente malo de 1 misión fatal en apenas 12 vuelos
    Dicho eso, el Space Shuttle tenía capacidad de tripulación mucho mayor de la que la mayoría de las misiones necesitaban. Podía llevar hasta 8 personas, a diferencia de las 3 del Apollo o Soyuz, y si consideras que la mayoría de las misiones soviéticas/Roscosmos, ESA y CNSA eran totalmente autónomas y no tripuladas, ni siquiera había tripulación expuesta al riesgo. Quizá esta analogía encaja mejor con Kubernetes: un sistema altamente diseñado, potente y multipropósito, pero que requiere mucha atención y probablemente se usa un poco más de lo necesario

  • Medido por pasajero-milla, que es la métrica más común, el Space Shuttle está entre los vehículos más seguros jamás construidos y volados
    Hablando honestamente como alguien cuya infancia fue exactamente en los años 80, no sé cómo podría no recordarse con cariño. ¿Será que son demasiado jóvenes y solo ven este programa y todas sus misiones y logros en retrospectiva, con una perspectiva ya teñida por la atmósfera actual centrada en contratistas espaciales privados?

  • La historia de Richard Hipp sobre llevar el código de SQLite a estándares aeronáuticos también es bastante interesante: https://corecursive.com/066-sqlite-with-richard-hipp/#testin...

    DO-178B. Es un estándar de calidad para productos aeronáuticos críticos para la seguridad... Las pruebas deben hacer que cada operación de bifurcación en el código binario resultante se ejecute al menos una vez y se tome al menos una vez... Nos tomó un año trabajando 60 horas por semana... Marcó una diferencia enorme. Durante los siguientes 8 o 9 años prácticamente no hubo bugs

  • Esta parte me recuerda a la verificación de exhaustividad en código TypeScript. Siempre intento usarla
    https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

    • El más reciente satisfies never es muy bueno para esto. También resulta útil cuando, por preferencia, usas cadenas de if else

    • También podría gustarte ts-pattern

      https://github.com/gvergnaud/ts-pattern

  • Si nos limitamos a los casos en que se agrega un else explícito a cada if que no es completamente trivial, me pregunto cuánto más simple habría sido este código si los autores de Kubernetes hubieran diseñado alrededor de pattern matching estructural en vez de bloques if/else
    Varios lenguajes principales que soportan pattern matching estructural tienen herramientas para verificar en tiempo de compilación si el match es exhaustivo, y solo eso ya puede ser una solución idiomática que aumenta la densidad de información del código

  • Discusión de 2018: https://news.ycombinator.com/item?id=18772873

  • Solo le eché una mirada rápida al código, pero sinceramente no se ve tan mal. Hay cosas que yo habría hecho distinto, pero he visto código mucho peor
    Al menos este código da la impresión de seguir una sola regla, de que todo fue escrito con intención y de que hay cierto método dentro de este caos. Lo escogería cualquier día por encima del típico revoltijo de estilos mezclados, código perezoso y estructura ilógica que he visto tantas veces

  • Me pregunto por qué, al inventar nuevas prácticas de "seguridad", se ignoran las mejores prácticas documentadas de ingeniería de software
    Los módulos de 2,000 líneas, los métodos de 200 líneas y los if anidados 3 o 4 niveles suelen considerarse dañinos. Los comentarios que solo dicen qué se hace y no por qué tampoco son útiles y tienden a desalinearse del código real. También se ve uso innecesario de nil. Incluso sin entrar en problemas más profundos como acoplamiento o el principio de responsabilidad única, estas cosas saltan a la vista

    • Si crees que estas cosas son dañinas, te recomiendo leer "John Carmack on Inlined Code"
      http://number-none.com/blow/john_carmack_on_inlined_code.htm...

      "El código de control de vuelo del cohete Armadillo tenía apenas unos pocos miles de líneas, así que tomé la función principal de tic y empecé a inlinear todas las subrutinas. No puedo decir que encontré un bug oculto que realmente pudiera causar un choque, pero sí encontré algunas variables que se configuraban varias veces y algunos flujos de control que parecían un poco sospechosos, y el código final terminó siendo más pequeño y limpio."

      Si Carmack encontró valor en este enfoque, no parece algo que deba descartarse apresuradamente. Los comentarios posteriores también valen la pena

      "En los años posteriores a escribir ese artículo, me volví mucho más positivo respecto a la programación puramente funcional, incluso en C/C++, dentro de límites razonables... cuando se vuelva demasiado difícil de manejar, busca una forma de extraer bloques como funciones puras"

    • A veces el caso es "No Hay Otra Manera(TM)"
      Un límite arbitrario de líneas suele producir fragmentación innecesaria. Si además sumas includes, licencias, código pegamento y comentarios, termina siendo un espagueti difícil de navegar. Si intentas mantener los métodos en 200 líneas dentro de código de alto rendimiento, el rendimiento puede desplomarse como el vuelo de Ícaro

Si lees los comentarios del código, puedes ver que este código se simplificó en un solo módulo y que se le incorporó una enorme cantidad de conocimiento práctico para hacerlo accesible y, más importante aún, sostenible. Para alguien que no conoce el lenguaje o la lógica, los comentarios que trazan un panorama de lo que hace el código son muy útiles. También lo son para uno mismo, porque seis meses después hasta tu propio código puede volverse ajeno.

Los comentarios son parte del código y de la base de código. Si modificas el código alrededor y no actualizas los comentarios al mismo tiempo, estás introduciendo un bug de documentación en el código. Que el compilador no los procese no significa que no sean una parte funcional. En esencia, los comentarios son conocimiento y notas de investigación incrustadas en el código, y al mantener el código que escribiste pueden valer más que el código que se ejecuta.

Las mejores prácticas no son leyes ni reglas rígidas, sino pautas. Deben aplicarse cuando encajan con la base de código; no hay que seguirlas ciegamente hasta terminar creando una base de código problemática. A veces hay que doblar las reglas e inventar algo uno mismo, y eso es totalmente aceptable si sabes lo que estás haciendo.

  • Durante bastante tiempo probé escribir de esta forma "segura", pero me generó muchos más bugs que el manejo de errores estilo ferroviario mediante retornos tempranos, y además tardé mucho más en corregirlos.
    Si agregas un else explícito a cada bloque if, la complejidad de tener que recordar el contexto actual se dispara. Me parece razonable cambiar esa regla por: "todo bloque condicional if o bien retorna temprano, o bien tiene un bloque else correspondiente". El patrón if (cond) { manejo especial } sin duda es mucho más riesgoso y hace más difícil razonar sobre él que un retorno temprano.

  • No existe una única colección oficial de mejores prácticas.
    La longitud de una función o la cantidad de líneas de código en un archivo no es intrínsecamente dañina ni beneficiosa. Cada lenguaje tiene su propia visión sobre cómo organizar el código, pero ninguno puede afirmar que eso sea "la mejor práctica". Go no es un lenguaje que prefiera dividir el código en muchos archivos pequeños.

  • Un método de 200 líneas no está mal por naturaleza. Si el código interno es lineal y mantiene el mismo nivel de abstracción, puede ser la mejor opción.
    La alternativa, crear 40 métodos de 5 líneas, puede ser peor. Tienes que ir saltando de un lado a otro para entender el conjunto y además podrías arruinar el orden de las llamadas. Hay 40! permutaciones posibles.

  • Este tipo de código parece un candidato ideal para migrarlo a un sistema declarativo, basado en reglas y orientado por tablas.
    Ese enfoque es más fácil de entender y de verificar que un código imperativo improvisado lleno de cláusulas if. Este tipo de código desordenado normalmente es una señal de que falta una abstracción.

    • La filosofía de Go, en esencia, se parece a escribir todo de forma bastante directa, como si trasladaras más o menos tal cual el código que habrías escrito en C, y a no intentar abstraer demasiado.