Это нарушение чистого кода для вызова метода init в конструкторе, как это - PullRequest
13 голосов
/ 23 апреля 2011

Моя проблема в приведенном ниже коде состоит в том, что параметр param для конструктора фактически не отображается непосредственно на поля экземпляра класса.Поля экземпляра получают значение из параметра, для которого я использую метод initalize.Кроме того, я делаю некоторые вещи, чтобы созданный объект мог использоваться непосредственно в следующем коде, например, вызывая drawBoundaries ().Я чувствую, что он делает то, что подразумевается под созданием (инициализацией) Canvas в абстрактном смысле.

Мой конструктор делает слишком много?Если я добавлю методы для явного вызова вещи в конструкторе извне, это будет неправильно.Пожалуйста, дайте мне знать ваши взгляды.

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);
}
...

PS: Извините, если это выглядит как глупый вопрос;мой код будет проверен гуру ООП, и я просто волнуюсь: -0

РЕДАКТИРОВАТЬ:

Я прочитал некоторые опасения по поводу переопределения методов в initalizeCanvas () -К счастью, эти методы являются частными и не вызывают никаких других методов.

В любом случае, после дальнейших исследований в сети мне это больше понравилось ... Надеюсь, вы, ребята, согласны !! ??

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;
}

Ответы [ 6 ]

12 голосов
/ 23 апреля 2011

Как правило, плохая идея для конструктора содержать нетривиальный код.Как правило, конструкторы должны максимально назначать предоставленные значения для полей.Если объект требует сложной инициализации, эта инициализация должна быть обязанностью другого класса (обычно factory ).См. Замечательную статью Мишко Хевери на эту тему: Недостаток: Конструктор выполняет настоящую работу .

2 голосов
/ 23 апреля 2011

Вы никогда не должны вызывать не финальные методы в конструкторе. Эффективная Java хорошо объясняет почему, но в основном ваш объект не находится в стабильном состоянии до возврата конструктора. Если ваш конструктор вызывает методы, которые переопределены подклассом, вы можете получить странное, неопределенное поведение.

Также см. этот ответ .

1 голос
/ 23 апреля 2011

Хотя это не самый элегантный способ сделать это, я не вижу в нем недостатков с точки зрения ОО.Однако, если вы не вызываете метод private initalizeCanvas из какого-либо другого класса, вы можете переместить эти три строки в сам конструктор.

0 голосов
/ 24 апреля 2011

Закрытая функция инициализации может быть полезна , если у вас есть несколько конструкторов, в которых некоторые параметры имеют значения по умолчанию или предполагаемые значения.После того, как все определено, одна частная функция init заполняет объект.

В классе с одним конструктором, вероятно, нет.

0 голосов
/ 23 апреля 2011

Это зависит.

Для конструктора неплохо вызывать закрытый метод, при условии, что закрытый метод не вызывает другие методы, которые могут быть переопределены.Однако, если метод или один из методов, которые он вызывает, могут быть переопределены, вы можете столкнуться с проблемой.В частности, метод переопределения будет вызываться перед запуском конструктора переопределяющих классов и будет видеть поля экземпляра до их инициализации.

Вторая проблема, если некоторые методы в вашем методе initalizeCanvas выглядят так, как будто они мои«опубликовать» текущий объект до того, как он будет полностью инициализирован.Это потенциально проблематично, если приложение является многопоточным, и может привести к тому, что другие потоки увидят устаревшие значения полей.

0 голосов
/ 23 апреля 2011

Я вижу две потенциальные проблемы:

  • Методы, которые вы вызываете в initializeCanvas, закрытые или окончательные?Если это не так, подкласс может переопределить их и невольно сломать ваш конструктор.

  • Вы выполняете графические операции в методе drawBoundaries?Хорошей практикой для конструктора является выполнение только минимума, необходимого для создания корректного объекта.Эти операции необходимы для того, чтобы холст имел правильное начальное состояние?

...