Evita que los desarrolladores usen constantes

7

Tengo un sistema de software que permite a los desarrolladores especificar un ID o nombre para crear NodeReferences . Ambos funcionan bien, pero no se garantiza que las ID sean iguales en diferentes entornos. He intentado en la documentación y en las conversaciones para enfatizar que las ID no deberían estar codificadas (ya que se romperán cuando se implementen en entornos diferentes), pero algunos desarrolladores todavía las están utilizando.

Esto funciona solo en el entorno dev:

var level = LevelReference.ById(20);
var node = NodeReference.ByName(level, _patientGuid.ToString());

Esto funciona en todos los entornos:

var app = ApplicationReference.ByName("Reporting");
var area = AreaReference.ByName(app, "Default");
var level = LevelReference.ByName(area, "Patient");
var node = NodeReference.ByName(level, _patientGuid.ToString());

Creo que el atractivo es que produce menos líneas de código. El uso de las ID no es malo en sí mismo (ya que existen casos de uso válidos como el almacenamiento en caché de la ID devuelta por el servidor y su uso posterior para una búsqueda más rápida), pero las ID codificadas son malas. La mayoría de las veces, el primer código lanzará una excepción, pero es posible que sea un ID válido para un objeto diferente al que pretendía el desarrollador, y esto podría ocasionar problemas muy graves.

¿Cuál es la mejor manera de desalentar el uso de tales constantes en el código? Idealmente, me gustaría lanzar algún tipo de error de compilación cuando vea un código como el primer ejemplo, o al menos lanzar una excepción antes de que la llamada llegue a la base de datos.

    
pregunta p.s.w.g 06.03.2013 - 17:54

7 respuestas

16

Oculte los int ids en objetos / estructuras opacas;

var id = level.id;

Aquí id es una estructura de este tipo.

De esta manera, puede eliminar el método ById(int) y reemplazarlo con ById(Id) y todavía le permite mantener el Id s cobrado para uso futuro.

También puede crear tipos de ID únicos para cada tipo de referencia para garantizar la seguridad del tipo (por lo que no puede hacer NodeReference.ById(level.id) ).

    
respondido por el ratchet freak 06.03.2013 - 18:01
17

Proporcionar una mejor API:

var node = NodeReference.byName("Reporting", "Default", "Patient", _patientGuid.ToString());

O proporcione funciones de utilidad para recuperar nodos importantes

var node = NodeReference.byName( theDefaultPatientNode(), _patientGuid.ToString() );

O tenga los distintos nodos en una enumeración, y haga algo como:

var node = NodeReference.byCategory( REPORT_PATIENT, _patientGuid.ToString() );

No podrás vender escribiendo 4 líneas de código para hacer algo tan simple. Intentan utilizar identificadores porque es demasiado difícil obtener el objeto relevante de lo contrario. ¡Así que hazlo más fácil!

    
respondido por el Winston Ewert 06.03.2013 - 18:25
8

Código como

var level = LevelReference.ById(20);

nunca debe pasar una revisión de código, ya que contiene números mágicos . Solo eso debería ser razón suficiente para prohibir tales prácticas.

Y si intentan evitarlo mediante la creación de constantes con nombre, el revisor debe preguntar al autor dónde obtuvo la garantía de que el número es correcto para todas las implementaciones.

    
respondido por el Bart van Ingen Schenau 06.03.2013 - 18:40
6

Aplasta su mano en la revisión del código.

Dicho esto, ¿hay una diferencia real entre LevelReference.ById() y LevelReference.ByName() ? En su ejemplo, está usando valores codificados en ambos.

En una base de código en la que he trabajado, manejamos una situación similar predefiniendo los valores "codificados" que se configuran con nuestro script de configuración de la base de datos y tenemos una enumeración que asigna un valor enumerado al valor de la base de datos. p>     

respondido por el Dave Rager 06.03.2013 - 18:01
3

Si entiendo correctamente, esos identificadores son las claves principales (columna de identidad) en la base de datos. En este caso, una solución alternativa sería no prohibir o rastrear las constantes en el código, sino simplemente aleatorizar esas claves en el script de implementación.

Dado que no se garantiza que las ID permanezcan iguales en los datos de prueba, la única solución para los desarrolladores sería pasar a algo más confiable: en su caso, nombres.

    
respondido por el Arseni Mourzenko 06.03.2013 - 18:01
3

Parece que está intentando encontrar una solución técnica para un problema social. Deberías llamarlo en revisiones de código. Haga algo en su entorno de prueba / ensayo para asegurarse de que los números de nodo nunca coincidan con un entorno de desarrollo limpio (tal vez inserte un número aleatorio de filas de basura en cada instalación) y devuélvalo al desarrollo como roto.

Si la gente insiste en tratar de solucionar esto, déjalos ir.

    
respondido por el Sean McSomething 06.03.2013 - 21:23
1

¡Doctor! ¡Doctor! ¡Me duele cuando hago esto!

Los desarrolladores están haciendo un mal uso de su API porque se los ha proporcionado. Si cree que LevelReference.ById() no es bueno de usar, no debe proporcionarlo. Si lo quitas, dejarán de usarlo :-)

Ya que estamos hablando de C #, también puedes marcar ById() como [Obsolete] y al menos recibirán una advertencia de desaprobación. Para aquellos que compilan con todas las advertencias-son-errores, eso es fatal.

    
respondido por el Ross Patterson 06.03.2013 - 23:46

Lea otras preguntas en las etiquetas