2009-03-11 22 views
10

considere el siguiente código:negocios código de la lógica de validación Olor

partial class OurBusinessObject { 
    partial void OnOurPropertyChanged() { 
     if(ValidateOurProperty(this.OurProperty) == false) { 
      this.OurProperty = OurBusinessObject.Default.OurProperty; 
     } 
    } 
} 

Es decir, cuando se cambia el valor de OurProperty en OurBusinessObject, si el valor no es válido, establece que es el valor por defecto. Este patrón me parece como código malicioso, pero otros aquí (en mi empleador) no están de acuerdo. ¿Cuáles son tus pensamientos?

Editado para agregar: Me han pedido que agregue una explicación de por qué esto está bien. La idea era que, en lugar de que los productores del objeto comercial validaran los datos, el objeto comercial podría validar sus propias propiedades y establecer valores predeterminados correctos en los casos en que fallara la validación. Además, se pensó que, si las reglas de validación cambian, los productores de objetos comerciales no tendrán que cambiar su lógica ya que el objeto comercial se encargará de validar y limpiar los datos.

Respuesta

17

Es absolutamente horrible. Buena suerte tratando de solucionar problemas en producción. Lo único que puede hacer es cubrir errores, que aparecerán en otro lugar, donde no será obvio de dónde provienen.

+0

Acepto en su mayor parte, pero algunas propiedades pueden ser predeterminadas de forma segura si se espera que los datos nocivos entren lentamente. Realmente no sabemos cuál es la situación aquí. –

+0

Si las propiedades pueden predeterminarse de manera segura, ¿por qué solicitarlas en primer lugar? ¡Estás ignorando lo que el usuario está proporcionando de todos modos! –

3

Creo que tengo que estar de acuerdo contigo. Esto definitivamente podría conducir a problemas donde la lógica inesperadamente regresa a los valores predeterminados, lo que podría ser muy difícil de depurar.

Por lo menos, este comportamiento se debe registrar, pero esto parece más como un caso para lanzar una excepción.

1

Deteste el término 'olor a código' aparte, puede que tenga razón; dependiendo de dónde provenga, cambiar el valor en silencio probablemente no sea algo bueno. Sería mejor asegurarse de que su valor sea válido en lugar de volver al valor predeterminado.

0

Puede o no "oler", pero me inclino más hacia "Sí huele".

¿El ajuste OurProperty a la configuración predeterminada tiene una razón lógica para hacerlo o simplemente es conveniente hacerlo en el código? Es posible, aunque poco probable en la práctica, idear un escenario en el que este sea el comportamiento esperado, pero supongo que en la mayoría de los casos debería lanzar una excepción y manejarla limpiamente en alguna parte.

Establece el valor predeterminado para acercarse o alejarse de la funcional especificaciones descripción de cómo se supone que la aplicación funcione?

3

Para mí, esto parece ser el síntoma, más que el problema real. Lo que realmente está pasando es que el setter para OurProperty no conserva el valor original para usar en el evento . Si lo hace, de repente es más fácil tomar mejores decisiones sobre cómo proceder.

Para el caso, lo que realmente desea es un evento OnOurPropertyChanging que se produce desde el setter antes de que realmente tiene lugar la tarea. De esta manera, puede permitir o denegar la tarea en primer lugar. De lo contrario, hay una pequeña cantidad de tiempo en que su objeto no es válido, y eso significa que el tipo no es seguro para subprocesos y no puede contar con consistencia si considera que la concurrencia es una preocupación.

+2

La aliteración es divertida. –

0

¿Está validando un cambio después de haberlo hecho? La validación debe hacerse antes de que se altere la propiedad de ocupación.

responder a sus quest: la solución presentada en ese fragmento de código puede generar grandes problemas en la producción, no se sabe si el valor por defecto apareció allí debido a una entrada no válida o simplemente porque otra cosa establecer el valor por defecto a la

2

Definitivamente una práctica cuestionable.

¿Cómo se asignaría un valor no válido a esta propiedad? ¿Eso no indicaría que hay un error en algún lugar del código de llamada, en cuyo caso probablemente quieras saber de inmediato? O que un usuario ingrese algo incorrectamente, en cuyo caso ellos deben ser informados de inmediato?

En general, "fallar rápido" hace que rastrear errores sea mucho más fácil. Silenciosamente asignar un valor predeterminado detrás de las escenas es similar a "magia" y solo causará confusión a quien tenga que mantener la base del código.

1

Recomiendo encarecidamente la refacturación para validar antes de configurar la propiedad.

Siempre se puede contar con un método que era más como:

T GetValidValueForProperty<T>(T suggestedValue, T currentValue); 

o incluso:

T GetValidValueForProperty<T>(string propertyName, T suggestedValue, T currentValue); 

Si lo hace, antes de establecer la propiedad, usted podría pasarlo al negocio lógica para validar, y la lógica de negocio podría devolver el valor de propiedad predeterminado (su comportamiento actual) O (más razonable en la mayoría de los casos), devolver el valor actual, por lo que la configuración no tuvo ningún efecto.

Esto se usaría más como:

 
T OurProperty 
{ 
    get 
    { 
     return this.propertyBackingField; 
    } 
    set 
    { 
     this.propertyBackingField = this.GetValidValueForProperty(value, this.propertyBackingField); 
    } 
} 

Realmente no importa lo que hagas, pero es importante para validar antes de cambiar su valor actual. Si cambia su valor antes de determinar si el nuevo valor es bueno, está pidiendo problemas a largo plazo.

0

Es difícil de decir sin conocer el contexto o las reglas comerciales. Sin embargo, en términos generales, solo debes validar en el momento de la entrada, y tal vez una vez más antes de la persistencia, pero la forma en que lo haces realmente no te permitirá validar dado que no permites que una propiedad contenga un valor no válido.

0

Creo que su lógica de validación debería generar una excepción si se solicita que utilice un valor no válido. Si su consumidor desea usar un valor predeterminado, debe solicitarlo explícitamente, ya sea a través de un valor especial documentado o mediante otro método.

El único tipo de excepciones que puedo pensar que serían perdonables sería, como, la normalización de casos, como en los campos de correo electrónico para detectar duplicados.

0

Además, ¿por qué en el mundo es este parcial? El uso de clases parciales para todo lo que no sea un marco de código generado es en sí mismo un código, ya que es probable que los uses para ocultar la complejidad que debe dividirse de todos modos.

+0

No estoy de acuerdo con que parcial sea intrínsecamente malo. cf. http://msdn.microsoft.com/en-us/library/bb738612.aspx para un uso. – jason

+0

Este objeto probablemente está hecho del diseñador EF, ya que dice que está usando EF (Etiquetas). En ese caso, las clases parciales son la mejor opción. –

0

Estoy de acuerdo con Grzenio y agregaría que la mejor manera de manejar un error de validación en la capa de dominio (también conocido como objetos comerciales) es generar una excepción. Esa excepción podría propagarse hasta la capa de la interfaz de usuario donde podría manejarse y rectificarse interactivamente con el usuario.Sin embargo, dependiendo de las capacidades y tecnologías involucradas, esto puede no ser siempre factible, en cuyo caso, probablemente debería estar validando en la capa UI (posiblemente además de la capa de dominio). No es ideal, pero podría ser tu única opción viable. En cualquier caso, establecerlo en un valor predeterminado es algo horrible de hacer y dará lugar a errores sutiles que serán casi imposibles de diagnosticar. Si se hace en una escala amplia, tendrá un sistema no-mantenido en muy poco tiempo (especialmente si no tiene pruebas de unidades que lo respalden).

0

Un argumento que tengo en contra de esto es el siguiente. Supongamos que el usuario/productor del objeto comercial ingresa accidentalmente un valor no válido. Entonces este patrón pasará por alto ese hecho y por defecto para limpiar datos. Pero la forma correcta de manejar esto es lanzar un error y hacer que el usuario/productor verifique/limpie sus datos de entrada.

0

Yo diría, implementar PropertyChanging y permitir que la lógica de negocio apruebe/niegue un valor, y luego, lanzar una excepción para los valores no válidos.

De esta forma, nunca tendrá un valor no válido. Eso, y nunca debes cambiar la información de un usuario. ¿Qué sucede si un usuario agrega una entrada a la base de datos y la sigue para sus propios registros? Su código reasignaría el valor al valor predeterminado, y ahora está rastreando la información incorrecta. Es mejor informar al usuario lo antes posible.

Cuestiones relacionadas