Refactor un validador de selección -- # campo con validation camp codereview Relacionados El problema

Refactor a selection validator


7
vote

problema

Español

Tengo una tabla de datos y tengo que validar cada campo en él. Tengo refactor este código a esto a continuación, pero la complejidad es 15 (!!) ¿Debo hacer algo como el diccionario con 9988776665544331 como la clave y 99887766555443322 como valor?

  private bool CheckField(DataRow dataRow, ValidationField validationField)         {             bool result = false;             if (validationField.Requiered)             {                 if (validationField.Type == typeof (int))                 {                     result = this.CheckIntegerAndNotNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof (DateTime))                 {                     result = this.CheckDateTimeAndNotNull(dataRow[validationField.Name].ToString());                 }             }             else             {                 if (validationField.Type == typeof (int))                 {                     result = this.CheckIntegerOrNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof(DateTime))                 {                     result = this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof(string))                 {                     result = this.CheckStringOrNull(dataRow[validationField.Name].ToString(),                         validationField.MaxLength.Value);                 }                 else if (validationField.Type == typeof(decimal))                 {                     result = this.CheckDecimalOrNull(dataRow[validationField.Name].ToString());                 }             }             return result;         }   public class ValidationField     {         public Type Type { get; set; }         public string Name { get; set; }         public bool Requiered  { get; set; }         public int? MaxLength { get; set; }     }   
Original en ingles

I have a data table and have to validate every field in it. I have refactor this code to this below, but the complexity is 15(!!) Should I make something like dictionary with type as Key and Func as Value?

private bool CheckField(DataRow dataRow, ValidationField validationField)         {             bool result = false;             if (validationField.Requiered)             {                 if (validationField.Type == typeof (int))                 {                     result = this.CheckIntegerAndNotNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof (DateTime))                 {                     result = this.CheckDateTimeAndNotNull(dataRow[validationField.Name].ToString());                 }             }             else             {                 if (validationField.Type == typeof (int))                 {                     result = this.CheckIntegerOrNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof(DateTime))                 {                     result = this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());                 }                 else if (validationField.Type == typeof(string))                 {                     result = this.CheckStringOrNull(dataRow[validationField.Name].ToString(),                         validationField.MaxLength.Value);                 }                 else if (validationField.Type == typeof(decimal))                 {                     result = this.CheckDecimalOrNull(dataRow[validationField.Name].ToString());                 }             }             return result;         }   public class ValidationField     {         public Type Type { get; set; }         public string Name { get; set; }         public bool Requiered  { get; set; }         public int? MaxLength { get; set; }     } 
     

Lista de respuestas

8
 
vote

La primera simplificación es para el verificación Required . Todo lo que necesita hacer aquí es verificar que el valor no sea nulo, no le importa el tipo. Luego, puede hacer el cheque específico de tipo más tarde. Así que retire el if...else y simplemente tenga:

  if (validationField.Requiered && DBNull.Value.Equals(validationField.Name]))     return false;   

La siguiente sección es un poco más difícil. Sin embargo, un enfoque sería algo así como:

  var dataString = dataRow[validationField.Name].ToString(); return dataString == Convert.ChangeType(dataString, validationField.Type).ToString();   

Lo que estamos haciendo aquí está usando el método 99887765555544334 para cambiar de la cadena a su tipo primario deseado. Luego, si la representación de la cadena del elemento convertido coincide exactamente con la cadena original, decimos que la cadena debe representar exactamente el elemento convertido, lo que significa que es del tipo de datos correcto. Es posible que desee pasar también en un 9988776665544335 por mayor seguridad.

Poner esos dos bloques de código juntos le daría mucho su método completo. Es posible que necesite una verificación adicional para la longitud máxima de una cadena, sin embargo.


Aparte de las alternativas anteriores, su código se ve en su mayoría bueno. Una cosa que haría es chatarra el result variable, y devuélvalo directamente desde dentro de su if sentencias. Los prefijos this.8 son innecesarios para los nombres de los métodos Pascalcased.


Como una nota de diseño más general, es posible que sea preferible realizar esta validación en un punto diferente en su proceso. Por ejemplo, DataColumn9 S puede configurarse para requerir un 99887777666555443310 if...else1 Propiedad, lo que significa que la tabla hará esta validación en gran medida para usted.

 

The first simplification is for the Required check. All you need to do here is check that the value is not null, you don't care about the type. Then you can do the type-specific check later. So remove the outermost if...else and simply have:

if (validationField.Requiered && DBNull.Value.Equals(validationField.Name]))     return false; 

The next section is a bit more difficult. However, one approach would be something like:

var dataString = dataRow[validationField.Name].ToString(); return dataString == Convert.ChangeType(dataString, validationField.Type).ToString(); 

What we're doing here is using .NET's Convert.ChangeType method to change from the string to your desired primative type. Then, if the string representation of the converted item exactly matches the original string, we say that the string must exactly represent the converted item, meaning it is of the correct data type. You may want to also pass in an IFormatProvider for additional safety.

Putting those two code blocks together would pretty much give you your full method. You may need one additional check for the max length of a string though.


Other than the above alternatives, your code mostly looks good. One thing I'd do is scrap the result variable, and return directly from inside your if statements. The this. prefixes are also unnecessary for PascalCased method names.


As a more general design note, it might be preferable to do this validation at a different point in your process. For example, DataColumns can be set to require a particular Type, and can also be told whether or not to accept null values using the AllowDBNull property, meaning the table will largely do this validation for you.

 
 
6
 
vote

Solo una pequeña nota sobre esto:

  if...else2  

No puedo decir si se supone que debe ser if...else3 o if...else4 . De cualquier manera, es un error tipográfico que necesita ser corregido.

 

Just a small note about this:

public bool Requiered  { get; set; } 

I can't tell if it's supposed to be Required or Requeried. Either way, it's a typo that needs to be corrected.

 
 
4
 
vote

Las afirmaciones de Si se ven realmente desordenadas para mí, preferiría poner esto en un interruptor y usar 2 declaraciones de operador de Ternary para que solo esté realizando los cálculos cuando necesito.

Uso del interruptor También pude deshacerme de la variable booleana y las asignaciones a esa variable, preferiría simplemente devolver el resultado de los "cheques"

Aquí está lo que acumulé.

  if...else5  

Otro pensamiento que tuve después de revisar este código, creo que debes cambiar el nombre de sus cheques a algo un poco más específico, casi me volvía más allá del hecho de que deberían devolver un resultado booleano, al menos supongo que deberían .

 

the if statements look really messy to me, I would much rather put this into a Switch and use 2 ternary operator statements so that I am only performing if calculations when I absolutely need to.

using the switch I was also able to get rid of the boolean variable and the assignments to that variable, I would much rather just return the result of the "checks"

Here is what I came up with.

private bool CheckField(DataRow dataRow, ValidationField validationField) {     switch validationField.Type     {         case typeof(int):             return validationField.Requiered                  ? this.CheckIntegerAndNotNull(dataRow[validationField.Name].ToString())                 : this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());             break;         case typeof(DateTime):             return validationField.Requiered                  ? this.CheckDateTimeAndNotNull(dataRow[validationField.Name].ToString())                  : this.CheckDateTimeOrNull(dataRow[validationField.Name].ToString());             break;         case typeof(string):             return this.CheckStringOrNull(dataRow[validationField.Name].ToString(),                 validationField.MaxLength.Value);             break;         case typeof(decimal):             return this.CheckDecimalOrNull(dataRow[validationField.Name].ToString());             break;         default :             return false             break;     } } 

Another thought that I had after reviewing this code, I think that you should rename your checks to something a little more specific, I almost blew right past the fact that they should return a boolean result, at least I assume that they should.

 
 

Relacionados problema

7  Entrada de usuario de fregado  ( Scrubbing user input ) 
Estoy tomando la entrada de usuario de un archivo en el formulario describe "Caesar" do latin_encrypter = Caesar.new 4 dna_encrypter = Caesar.new 3, 'AC...

3  Verificando que todos los campos de formulario se completan  ( Verifying that all form fields are filled in ) 
Agarré la forma de algún sitio aleatorio porque solo estoy interesado escribir el Javascript en este momento. Estoy tratando de verificar que un usuario hay...

5  Validación del formulario AJAX  ( Ajax form validation ) 
Este es mi primer intento de la validación del formulario AJAX, y todo funciona como se esperaba, el ejemplo final final mantendrá muchos más campos de entrad...

6  Validando a ISBNS  ( Validating isbns ) 
Solo estoy buscando una manera más limpia. Obviamente, podría almacenar una variable para algunas partes 'AA2'-> {'A', '2', 'A2', 'AA2'}2 . Personalmente, cr...

3  Inicialización de propiedades numéricas de un archivo de propiedades basadas en texto  ( Initialising numerical properties from a text based properties file ) 
Este es un instantáneo de datos de DataSource basado en el marco de primavera que lee sus propiedades del contexto marco de resorte, es decir, los valores son...

2  Validación instantánea de la longitud del campo de la forma  ( Instantaneous validation of form field length ) 
Dado que tengo una función que se puede hacer de unas maneras como de costumbre, me preguntaba qué sigue es el mejor para lograr la validación del formulario ...

2  Validando múltiples parámetros por regex y longitud  ( Validating multiple parameters by regex and length ) 
Tengo un conjunto de funciones de validación que devuelven un código de respuesta junto con un mensaje de error si la validación falla. Las solicitudes pueden...

2  Función de validación de forma para reaccionar  ( Form validation function for react ) 
Necesito validar varios campos de entrada usando reaccionar. Estoy usando un bloque simple if-común para cada campo de entrada, pero me gustaría optimizar mi ...

6  Limitar datos en Java idiomático  ( Constraining data in idiomatic java ) 
JUGANDO CON UN SIMPLE JAVA <?php //class sql //{ class Result { private $m_handle; private $m_statement; function __construc...

2  Formulario de clases de clase y clases infantiles  ( Form validator class and child classes ) 
Estoy creando una clase de formulario para validar los datos del formulario, y quisiera algunos consejos sobre cómo refactorizarlo por las mejores prácticas. ...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos