2008-11-06 14 views
8

Estoy implementando una clase para comparar árboles de directorios (en C#). Al principio implementé la comparación real en el constructor de la clase. De esta manera:¿Se considera un mal diseño realizar operaciones largas en un constructor?

DirectoryComparer c = new DirectoryComparer("C:\\Dir1", "C:\\Dir2"); 

Pero no se siente "bien" hacer una operación larga posible en el constructor. Una forma alternativa es hacer que el constructor sea privado y agregar un método estático como este:

DirectoryComparer c = DirectoryComparer.Compare("C:\\Dir1", "C:\\Dir2"); 

¿Qué opinas? ¿Esperas que un constructor sea "rápido"? ¿El segundo ejemplo es mejor o solo está complicando el uso de la clase?

Por cierto:

no voy a marcar cualquier respuesta como aceptada porque no creo que haya una respuesta correcta, sólo preferencia y gusto.

Editar:

sólo para aclarar mi ejemplo un poco. No solo estoy interesado si los directorios difieren, también estoy interesado en cómo difieren (qué archivos). Entonces un valor de retorno int simple no será suficiente. La respuesta de cdragon76.myopenid.com en realidad está bastante cerca de lo que quiero (+1 para ti).

+1

Si no marca una respuesta, ¿quizás debería ser una wiki de la comunidad? –

+0

Acepto [10chars] –

Respuesta

10

Prefiero el segundo.

Espero que el constructor instanciar la clase. El método de comparación hace lo que está diseñado para hacer.

3

Nunca debe hacer nada que pueda fallar en un constructor. No desea crear nunca objetos no válidos. Si bien podría implementar un estado "zombie" donde el objeto no hace mucho, es mucho mejor realizar cualquier lógica compleja en métodos separados.

+0

Está perfectamente bien que un constructor falle y se lanza una excepción. Hay muchos ejemplos de esto en el marco. Debe asegurarse de que no se filtre una referencia a sí mismo durante el constructor, pero aparte de eso, está bien. No es que me gusten los constructores complejos, fíjate. –

+0

No creo que solo porque esté en el marco esté bien. El marco es genial, pero no es perfecto. Es un gran consejo decir que nunca debes hacer cosas que pueden fallar en un constructor. Pero sí, los constructores deberían poder lanzar excepciones en el caso de datos inválidos, en mi humilde opinión. – James

2

Sí, normalmente un constructor es algo rápido, está diseñado para preparar el objeto para su uso, no para realizar operaciones realmente. Me gusta tu segunda opción ya que la mantiene en una operación de una línea.

También podría hacerlo un poco más fácil al permitir que el constructor pase las dos rutas, y luego tener un método Compare() que realmente haga el procesamiento.

5

Creo que una interfaz puede ser lo que buscas. Crearía una clase para representar un directorio y haría que implementara la interfaz DirectoryComparer. Esa interfaz incluiría el método de comparación. Si C# ya tiene una interfaz Comparable, también podría implementarla.

En el código, la llamada sería:

D1 = new Directory("C:\"); 
.. 
D1.compare(D2); 
1

me gusta el segundo ejemplo porque explica lo que está ocurriendo exactamente cuando una instancia del objeto. Además, siempre uso el constructor para inicializar todas las configuraciones globales de la clase.

12

Creo que una combinación de ambas es la opción "correcta", ya que esperaría que el método Compare devuelva el resultado de la comparación, no el comparador mismo.

DirectoryComparer c = new DirectoryComparer(); 

int equality = c.Compare("C:\\Dir1", "C:\\Dir2"); 

... y como menciona Dana, existe una interfaz IComparer en .Net que refleja este patrón.

El método IComparer.Compare devuelve un int ya que el uso de las clases de IComparer es principalmente con la clasificación. El patrón general se ajusta a pesar de que el problema de la pregunta en que:

  1. Constructor inicializa una instancia con (opcionalmente) "configuración" parámetros
  2. Comparar método toma dos parámetros de "datos", los compara y devuelve un "número "

Ahora, el resultado puede ser un int, un bool, una colección de diffs. Lo que sea que se ajuste a la necesidad.

1

Creo que para un comparador de propósito general es posible que en la construcción sólo desee especificar los archivos que se están comparando y luego comparar más tarde- esta manera también se puede implementar lógica extendida:

  • Compare nuevo- ¿y si el directorios cambiados?
  • Cambie los archivos que está comparando al actualizar los miembros.

Además, es posible que desee considerar en su implementación recibir mensajes de su sistema operativo cuando se hayan modificado los archivos en los directorios de destino y, opcionalmente, volver a compilar.

El punto es que usted está imponiendo límites al asumir que esta clase solo se usará para comparar una vez para una sola instancia de esos archivos.

Por lo tanto, prefiero:

DirectoryComparer = new DirectoryComparer(&Dir1,&Dir2);

DirectoryComparer->Compare();

O

DirectoryComparer = new DirectoryComparer();

DirectoryComparer->Compare(&Dir1,&Dir2);

+0

Estoy de acuerdo, y todavía puede tener una sola línea diciendo nuevo DirectoryComparer (a, b) .Compare() –

+0

@Don: Un antiguo compañero de trabajo dijo que "intentaba hacer demasiadas cosas sexys en una línea". Si el nuevo falla por alguna razón, acaba de generar una excepción que puede ser difícil de rastrear. – tloach

0

Si está trabajando con C#, puede utilizar los métodos de extensión para crear un método para comparar 2 directorios que se adjuntaría a la compilación en DirectoryClass, por lo que se vería algo así como:

Directory dir1 = new Directory("C:\....."); 
Directory dir2 = new Directory("D:\....."); 

DirectoryCompare c = dir1.CompareTo(dir2); 

Esto sería una implementación mucho más clara. Más sobre los métodos de extensión here.

0

Si una operación puede llevar un tiempo desconocido, es una operación que quizás desee exportar a un hilo diferente (para que su hilo principal no se bloquee y pueda hacer otras cosas, como mostrar un indicador de progreso giratorio para ejemplo). Es posible que otras aplicaciones no quieran hacerlo, es posible que deseen todo dentro de un solo hilo (por ejemplo, aquellos que no tienen UI). Mover la creación de objetos a un hilo separado es un poco incómodo en mi humilde opinión. Prefiero crear el objeto (rápidamente) en mi hilo actual y luego simplemente dejar que un método se ejecute dentro de otro hilo y una vez que el método termine de ejecutarse, el otro hilo puede morir y puedo tomar el resultado de este método en mi hilo actual usando otro método del objeto antes de volcar el objeto, ya que estoy contento tan pronto como sé el resultado (o manteniendo una copia si el resultado involucra más detalles que puedo tener que consumir de a uno por vez).

3

Estoy de acuerdo con el sentimiento general de no hacer operaciones largas dentro de los constructores.

Además, mientras que en el tema del diseño, consideraría cambiar su segundo ejemplo para que el método DirectoryComparer.Compare devuelva algo más que un objeto DirectoryComparer. (Tal vez una nueva clase llamada DirectoryDifferences o DirectoryComparisonResult.) Un objeto de tipo DirectoryComparer suena como un objeto que usaría para comparar directorios en lugar de un objeto que representa las diferencias entre un par de directorios.

Luego, si quiere definir diferentes formas de comparar directorios (como ignorar marcas de tiempo, atributos de solo lectura, directorios vacíos, etc.) puede hacer que esos parámetros pasen al constructor de clase DirectoryComparer. O bien, si siempre quiere que DirectoryComparer tenga exactamente las mismas reglas para comparar directorios, simplemente podría hacer que DirectoryComparer sea una clase estática.

Por ejemplo:

DirectoryComparer comparer = new DirectoryComparer(
    DirectoryComparerOptions.IgnoreDirectoryAttributes 
); 
DirectoryComparerResult result = comparer.Compare("C:\\Dir1", "C:\\Dir2"); 
0

Si los argumentos son sólo va a ser procesada una vez, entonces no creo que pertenecen ya sea como argumentos de constructor o estado de la instancia.

Sin embargo, si el servicio de comparación admite algún tipo de algoritmo suspendible o desea notificar a los oyentes cuando el estado de igualdad de dos directorios cambia en función de los eventos del sistema de archivos o algo así. Luego, los directorios forman parte del estado de la instancia.

En ninguno de los casos el constructor está haciendo otro trabajo que no sea la inicialización de una instancia. En el caso de dos, el algoritmo es manejado por un cliente, como un iterador, por ejemplo, o es impulsado por el hilo de escucha del evento.

Por lo general, trato de hacer cosas como esta: No mantenga estado en la instancia si se puede pasar como argumentos a los métodos de servicio. Intenta diseñar el objeto con un estado inmutable. La definición de atributos, como los que se usan en equals y hashcode siempre debe ser inmutable.

Conceptualmente, un constructor es una función que mapea una representación de objetos para el objeto que representa.

Según la definición anterior, Integer.valueOf (1) es en realidad más un constructor que un nuevo Integer (1) porque Integer.valueOf (1) == Integer.valueOf (1). , En cualquier caso, este concepto también significa que todos los argumentos del cosntructor, y solo el argumento del constructor, deberían definir el comportamiento igual de un objeto.

0

Definitivamente haré el segundo.

Las acciones largas en un constructor están bien si realmente están construyendo el objeto para que pueda usarse.

Ahora una cosa que veo que la gente hace en los constructores es llamar a los métodos virtuales. Esto es MALO, ya que una vez que alguien lo utiliza como una clase base y anula una de esas funciones, usted llamará a la versión de la clase base no a la derivada una vez que ingrese a su constructor.

0

No creo que hablar de términos abstractos como "longy" tenga algo que ver con la decisión si pones algo en un constructor o no.

Un constructor es algo que se debe utilizar para inicializar un objeto, un método se debe utilizar para "hacer algo", es decir, tener una función.

1

Creo que no solo es correcto para un constructor tomar tanto tiempo como sea necesario para construir un objeto válido, sino que se requiere que el constructor lo haga. La postergación de la creación de objetos es muy mala ya que terminas con objetos potencialmente no válidos. Por lo tanto, deberá verificar un objeto cada vez antes de tocarlo (así es como se hace en el MFC, tiene los métodos bool IsValid() en todas partes).

Solo veo una ligera diferencia en las dos formas de crear el objeto. Uno puede ver al nuevo operador como una función estática de la clase de todos modos. Entonces, todo esto se reduce al azúcar sintáctico.

¿Qué significa la clase DirectoryComparer? ¿Cuál es su responsabilidad? Desde mi punto de vista (que es la vista de un programador de C++) parece que sería mejor usar una función gratuita, pero no creo que puedas tener funciones gratuitas en C#, ¿o sí? Supongo que recogerás los archivos que son diferentes en el objeto DirectoryComparer. Si es así, mejor podría crear algo así como una matriz de archivos o una clase equivalente que se llame en consecuencia.

Cuestiones relacionadas