2011-04-23 25 views
13

Mi preocupación en el siguiente código es que el parámetro para el constructor no se correlaciona directamente con los campos de instancia de la clase. Los campos de instancia derivan valor del parámetro y para el cual estoy usando el método initalize. Además, hago algunas cosas para que el objeto creado se pueda usar directamente en el código que sigue, p. llamando a drawBoundaries(). Siento que está haciendo lo que se entiende por crear (inicializar) un Lienzo en un sentido abstracto.¿Es una violación de Clean Code llamar al método init en un constructor como este?

¿Mi constructor está haciendo demasiado? Si agrego métodos para llamar las cosas en el constructor explícitamente desde afuera, eso será incorrecto. Por favor, hágame saber sus puntos de vista.

public class Canvas { 

private int numberOfRows; 
private int numberOfColumns; 
private final List<Cell> listOfCells = new LinkedList<Cell>(); 

public Canvas(ParsedCells seedPatternCells) { 
    initalizeCanvas(seedPatternCells); 
} 

private void initalizeCanvas(ParsedCells seedPatternCells) { 
    setNumberOfRowsAndColumnsBasedOnSeedPatten(seedPatternCells); 
    drawBoundaries(); 
    placeSeedPatternCellsOnCanvas(seedPatternCells); 
} 
... 

P.S .: Lo siento si esto parece una pregunta tonta; mi código va a ser revisado por un gurú de la programación orientada a objetos y estoy preocupada: -0

EDIT:

leí algunas preocupaciones acerca de los métodos de initalizeCanvas() ser invalidados - afortunadamente estos métodos son privado y no llame a ningún otro método.

De todos modos, después de seguir investigando en la red, he empezado a gustarme más ... ¡¡Espero que estén de acuerdo !! ??

public class Canvas { 

private int numberOfRows; 
private int numberOfColumns; 
private final List<Cell> listOfCells = new LinkedList<Cell>(); 

private Canvas() { 
} 

public static Canvas newInstance(ParsedCells seedPatternCells) { 
    Canvas canvas = new Canvas(); 
    canvas.setNumberOfRowsAndColumnsBasedOnSeedPatten(seedPatternCells); 
    canvas.drawBoundaries(); 
    canvas.placeSeedPatternCellsOnCanvas(seedPatternCells); 
    return canvas; 
} 
+1

No veo nada discutible con su código. –

Respuesta

11

En general, es una mala idea que un constructor contenga un código no trivial. Como regla general, los constructores deben, como máximo, asignar valores suministrados a los campos. Si un objeto requiere una inicialización compleja, esa inicialización debería ser responsabilidad de otra clase (generalmente, una factory). Vea la gran reseña de Miško Hevery sobre este tema: Flaw: Constructor does Real Work.

1

Aunque no es la forma más elegante de hacerlo, no lo veo como defectuoso desde la perspectiva OO. Sin embargo, si no llama al método privateinitalizeCanvas desde cualquier otro lugar dentro de la clase, entonces puede considerar mover esas tres líneas al propio constructor.

0

veo dos problemas potenciales:

  • son los métodos que llamas en initializeCanvas privado o final? Si no lo son, es posible que una subclase los anule y sin querer rompa su constructor.

  • ¿Estás haciendo operaciones gráficas en el método drawBoundaries? Es una buena práctica para un constructor hacer solo el mínimo necesario para crear un objeto válido. ¿Son necesarias esas operaciones para que el lienzo tenga un estado inicial válido?

2

Nunca debe llamar a métodos no finales en un constructor. Efectivo Java hace un buen trabajo explicando por qué, pero básicamente su objeto no está en un estado estable antes de que el constructor regrese. Si su constructor llama a métodos que son reemplazados por una subclase, puede obtener un comportamiento extraño e indefinido.

Ver también this answer.

+1

en este caso, el método es 'private', por lo que no podría sobrescribirse por subclase. ¿Me estoy perdiendo de algo? – kunal

+0

Sí, su método privado está llamando a otros métodos. ¿Son todos esos métodos privados y/o finales? –

+0

Tuve una entrevista de trabajo hace un par de años, donde me preguntaron qué le pasaba a un código. Después de mencionar los más obvios, les dije que había un posible problema de diseño, porque como regla general nunca llamamos a los métodos privados de un constructor. - Esto se intensificó en una discusión de media hora, porque el entrevistador simplemente no me creyó. Al final, no me dieron el trabajo. Aparentemente él tomó esto personal. Según el entrevistador: "No entiendo OOP". :-) - Pero es bueno tener una referencia a un libro como "Effective Java". Eso hubiera sido una clara victoria. – bvdb

0

Depende.

No es malo para un constructor llamar a un método privado, siempre que el método privado no invoque otros métodos que podrían anularse. Sin embargo, si se puede anular el método o uno de los métodos que llama, puede tener problemas. Específicamente, se llamará al método de anulación antes de que se ejecute el constructor de las clases de anulación y verá los campos de instancia antes de que se hayan inicializado.

Un segundo problema si parece que algunos de los métodos del método initalizeCanvas mi "publican" el objeto actual antes de que se haya inicializado por completo.Esto es potencialmente problemático si la aplicación tiene varios subprocesos y podría dar lugar a que otros subprocesos vean valores de campo obsoletos.

0

Una función de inicialización privada puede ser útil si tiene múltiples constructores donde algunos parámetros están predeterminados o no. Una vez que se ha determinado todo, una función de inicio privada rellena el objeto.

En una clase de un constructor, probablemente no.

Cuestiones relacionadas