2010-02-24 8 views
6

Creo que me estoy volviendo loco, por favor tranquilízame.Métodos que envuelven un solo método

public class MyFile 
{ 
    public static byte[] ReadBinaryFile(string fileName) 
    { 
     return File.ReadAllBytes(fileName); 
    } 

    public static void WriteBinaryFile(string fileName, byte[] fileContents) 
    { 
     File.WriteAllBytes(fileName, fileContents); 
    } 
} 

La gente sigue en la adición de un código como lo anterior en nuestra base de código, sin duda esto está mal y horrible y estoy haciendo un favor al mundo mediante su eliminación y sustitución de todos (o ambos, en este caso ...) referencias a él con el código interno.

¿Hay alguna justificación real para este tipo de cosas? ¿Podría perderme la imagen más grande? Estamos bastante YAGNI -centric en nuestro equipo y esto parece ir en contra de eso. Pude entender si este fue el comienzo de algo más, sin embargo, este código ha estado inactivo por muchos meses hasta que tropecé con él hoy. Cuanto más busco, más encuentro.

Respuesta

10

Como está escrito, la clase/métodos son basura. Sin embargo, puedo ver una situación en la que un similares patrón podría usarse legítimamente:

public interface IFileStorage 
{ 
    byte[] ReadBinaryFile(string fileName); 
    void WriteBinaryFile(string fileName, byte[] fileContents); 
} 

public class LocalFileStorage : IFileStorage { ... } 

public class IsolatedFileStorage : IFileStorage { ... } 

public class DatabaseFileStorage : IFileStorage { ... } 

En otras palabras, si usted quiere apoyar a los diferentes tipos de almacenamiento, entonces es posible que en realidad envolver métodos muy sencillos con el fin para implementar una abstracción genérica.

Como está escrito, sin embargo, la clase no implementa ninguna interfaz, y los métodos son estáticos, por lo que es bastante inútil. Si intentas soportar el patrón anterior, entonces refactoriza; de lo contrario, deshazte de él.

+2

esta es una forma mucho mejor de hacerlo: D – Jimmy

+0

idea interesante, aplausos. – gingerbreadboy

4

La existencia de esos métodos es una aberración repugnante que debe corregirse inmediatamente.

Probablemente fueron escritas por alguien que no conocía la clase File, luego reescrito por alguien que lo hizo, pero no fue tan atrevido como usted.

+1

Si pudiera se obtendría dos votos, uno para "aberración repugnante" y otra para "audaz" :) – gingerbreadboy

+1

¿Por qué este downvoted? – SLaks

0

Supongo que la respuesta a esto depende de su equipo, prácticas y para qué sirve el propósito de los códigos (por ej., La pieza de código que ha encontrado actualmente escribe en un archivo pero estará escribiendo a un servicio web/máquina de base de datos/código Morse una vez que haya terminado, aunque esa "excusa" es un poco derrotada por los nombres de clase/método). Creo que usted respondió la pregunta con "Estamos bastante centrados en YAGNI en nuestro equipo y esto parece ir en contra de eso".

Sin embargo, la respuesta definitiva sería preguntarle a la persona que lo escribió por qué lo escribió de esa manera.

6

Es bastante tonto, hasta que piense en ocultar los detalles de implementación de esos métodos.

Tomemos, por ejemplo si usted tenía un código como éste

File.WriteAllBytes(fileName, fileContents); 

dispersos en todo el código, lo que si algún día abajo de la línea que quería cambiar el método de la aplicación de la escritura de un archivo a cabo? Bueno, en ese caso tendrías que recorrer todo tu código y actualizar todas estas líneas de código para adoptar el nuevo método, donde al igual que con la versión anterior, solo necesitarías cambiarlo en un solo lugar.

No estoy diciendo que su camino es el correcto y que está mal para corregirlo, sólo estoy añadiendo un poco de perspectiva

+2

El miedo a los cambios improbables contamina las bases de código: hace que sea más difícil de entender y mantener. –

+0

Buen punto. Tener un punto centralizado para escribir en el disco podría cosechar recompensas si necesitamos hacer un cambio en la base del código. Aunque no creo que esa haya sido la motivación aquí;) – gingerbreadboy

+0

Entiendo, y estoy de acuerdo, simplemente agregando lo que creo que estaba en la mente de esos desarrolladores – Jimmy

2

Si todos los métodos que hacen es tomar la misma lista de parámetros y los pasa a través, entonces no, no tiene sentido, y creo que realmente hace que el código sea menos comprensible.Sin embargo, si el método también transmite valores predeterminados además de los pasados ​​como parámetros, o realiza algún tipo de validación común en los parámetros, entonces es un argumento más difícil de hacer.

¿Están estos métodos marcadores de posición para agregar lógica posterior? Si es así, entonces los comentarios o incluso la temida declaración TODO deberían agregarse para que alguien se ajuste más tarde y complete el pensamiento.

+0

+1 para valores predeterminados si es necesario. –

2

No creo que estos métodos tengan mucho sentido como métodos estáticos de una clase auxiliar, pero el patrón tiene mérito en algunos casos. Por ejemplo:

  1. para desacoplar el código de una clase estática. Si MyFile no era estático, podría servir como una abstracción sobre el objeto estático IO.File. El código que accede directamente al sistema de archivos puede ser muy difícil de probar, por lo que las abstracciones de este tipo pueden ser útiles. (Por ejemplo, tengo una interfaz IFileSystem que toda mi código de E/S depende de, y mi clase FileSystem implementa esta interfaz y envuelve los métodos básicos de File.)

  2. Para preservar niveles consistentes de abstracción en un método. No me gusta mezclar código en el dominio de problema (por ejemplo, "clientes" y "pedidos") con código en el dominio _solution) (archivos y bytes y bits). A menudo escribiré envoltorios como este para hacer que el código sea más fácil de leer y para darme puntos de extensibilidad. Prefiero leer GetDataFileContents() que File.ReadAllBytes("foo.dat").

  3. Proporcionar el contexto. Si se está ejecutando un fragmento de código por sus efectos secundarios, como eliminar un archivo de texto para efectuar la eliminación del pedido de un cliente, entonces preferiría leer DeleteCustomerOrderFile("foo.txt") que File.Delete("foo.txt"). El primero proporciona información contextual al código, el último no.

+0

El punto 1 es lo que realmente quiero hacer con esta clase a largo plazo para ayudar a probar/burlarse, etc., pero la prioridad de hoy no lo es. – gingerbreadboy

Cuestiones relacionadas