«Классы никогда не должны выполнять работу с зависимостями в своих конструкторах».
Итак, цитата взята из "Внедрение зависимостей в .NET" . Имея это в виду, правильно ли спроектирован следующий класс?
class FallingPiece { //depicts the current falling piece in a tetris game
private readonly IPieceGenerator pieceGenerator;
private IPiece currentPiece;
public FallingPiece(IPieceGenerator pieceGenerator) {
this.pieceGenerator = pieceGenerator;
this.currentPiece = pieceGenerator.Generate(); //I'm performing work in the constructor with a dependency!
}
...
}
Итак, этот класс FallingPiece отвечает за управление текущей падающей частью в игре тетрис. Когда кусок достигает дна или другого места, вызывает событие, сигнализирующее о том, что затем через фабрику генерируется еще один новый кусок, который снова начинает падать сверху.
Единственная альтернатива, которую я вижу, - это иметь метод Initialize (), который будет генерировать кусок, но IMO, который отправляет немного против идеи заставить конструктор перевести ваш объект в правильное состояние.
6 ответов
В общем, эмпирические правила — это именно эмпирические правила. Не неизменные законы, от которых никогда не следует отклоняться — я почти уверен, что вы найдете случаи, когда выполнение действий с вашими зависимостями в вашем конструкторе имеет больше смысла, чем в противном случае.
Имея это в виду, давайте вернемся к вашему конкретному дизайну:
Итак, этот класс FallingPiece имеет ответственность за контроль над текущая падающая фигура в тетрисе игра. Когда кусок достигает дна или какое-то другое место, вызывает событие сигнализируя об этом, а затем, через фабрика, генерирует еще одну новую деталь что снова начинает падать сверху.
Мне кажется странным, что FallingPiece запускает генератор кусков после его завершения.
Я бы спроектировал это примерно так:
class PieceGenerator
{
public FallingPiece NextFallingPiece()
{
FallingPiece piece = new FallingPiece(NextPiece());
return piece;
}
// ...
}
class GameEngine
{
public PieceGenerator pieceGenerator = new PieceGenerator();
public void Start()
{
CreateFallingPiece();
}
void CreateFallingPiece()
{
FallingPiece piece = pieceGenerator.NextFallingPiece();
piece.OnStop += piece_OnStop;
piece.StartFalling();
}
void piece_OnStop(FallingPiece piece)
{
piece.OnStop -= piece_OnStop;
if (!GameOver())
CreateFallingPiece();
}
}
По крайней мере, в этом дизайне GameEngine полностью отвечает за сообщение Генератору, когда создавать свои части, что кажется более идиоматичным, чем FallingPiece с эта обязанность.
Да, я бы сказал, что здесь что-то не так. Трудно сказать, так как вы не показываете, как FallingPiece используется или как он вписывается в систему, но мне непонятно, зачем ему нужно получать currentPiece в его конструкторе.
Поскольку похоже, что currentPiece может меняться со временем, создается впечатление, что вы используете один экземпляр FallingPiece в качестве одноэлементного контейнера для текущего падающего фрагмента. В этом случае вы должны каким-то образом получить вторую, третью и так далее части. Почему бы просто не сделать то же самое с первой фигурой?
Вы говорите, что когда фигура достигает дна, возникает событие, запускающее генерацию новой фигуры, которая начинает падать. Разве вы не должны сами запускать это событие, чтобы начать первую часть?
В общем:
Общая причина (на мой взгляд) того, что работа с зависимостями не должна выполняться в конструкторе, заключается в том, что построение объект должен быть связан исключительно с настройкой класса, чтобы он мог делать то, что ему нужно. То, что классы действительно делают то, что они делают, должно выполняться через вызовы API объекта после его создания. В случае с FallingPiece, когда у него есть IPieceGenerator, он может делать все, что нужно. На самом деле это (начало падения куска) должно быть сделано путем вызова метода в его API или (как здесь, кажется, в этом случае) запускает событие, на которое он будет реагировать.
Я бы сказал, что этот класс спроектирован правильно, потому что для списка параметров конструктора требуется IPieceGenerator. Поэтому это не противоречит идее, однако, если бы это был вызов статического метода, не содержащегося в списке параметров, я бы сказал, что это не соответствует этому правилу.
Как отмечали другие, это утверждение действительно является эмпирическим правилом. Однако это довольно сильное правило, потому что оно дает нам более одного преимущества:
- Оно применяет SRP к конструктору; другими словами, он сохраняет конструктор максимально чистым, потому что он делает только одну вещь.
- Это упрощает модульное тестирование, поскольку создание SUT не включает в себя сложную Fixture Setup.
- Это делает проблемы дизайна более очевидными.
Это третье преимущество, на мой взгляд, играет здесь роль. Как уже было сказано, этот конструктор кричит о нарушении SRP. За что отвечает класс FallingPiece? Прямо сейчас кажется, что создают новые части и представляют падающую часть.
Почему FallingPiece не является реализацией IPiece?
В целом, мне кажется, что IPiece должен быть конечным автоматом. Сначала он движется, и им можно манипулировать, но когда он достигает дна, он привязывается к другим частям, пока его нельзя будет удалить. Вероятно, имеет смысл вызывать события при изменении состояния.
С объектно-ориентированной точки зрения, я думаю, игровая доска должна иметь набор экземпляров IPiece, которые знают, где они находятся и что они могут делать.
Подводя итог, я думаю, что правило не выполнять работу в конструкторе остается в силе, поскольку оно привлекает наше внимание к проблемам проектирования.
Если вы используете внедрение конструктора, это нормально.
Должно быть очевидно, почему это не будет работать с инъекцией сеттера. Я считаю, что цитата вполне верна с точки зрения последнего.
Оставление currentPiece нулевого значения не обязательно означает, что объект остается в унифицированном состоянии.
Например, вы можете преобразовать currentPiece в свойство и лениво инициализировать его при первом доступе.
Это никак не повлияет на общедоступный интерфейс, но сделает конструктор легковесным, что, как правило, хорошо.