"Классы никогда не должны выполнять работу с зависимостями в своих конструкторах".

Итак, цитата исходит из "Injection Dependency in.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 несет ответственность за контроль текущей падающей части в игре с тетрисом. Когда кусок попадает на дно или в другое место, возникает сообщение о событии, которое затем через factory генерирует еще одну новую фигуру, которая снова начинает падать сверху.

Единственная альтернатива, которую я вижу, это иметь метод Initialize(), который будет генерировать часть, но IMO, которая немного противоречит идее о том, что конструктор помещает ваш объект в допустимое состояние.

Ответы

Ответ 1

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

С учетом этого позвольте пересмотреть ваш конкретный дизайн:

Итак, этот класс FallingPiece имеет ответственность за текущая падающая деталь в тетрисе игра. Когда кусок попадает в нижнюю часть или другое место, вызывает событие сигнализируя, что, а затем, через factory, генерирует еще одну новую часть который начинает падать снова сверху.

Мне кажется странным, что 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, имеющий эту обязанность.

Ответ 2

Да, я бы сказал, что там что-то не создано. Трудно сказать, так как вы не показываете, как используется FallingPiece или как он вписывается в систему, но мне не имеет смысла, почему он должен получить currentPiece в своем конструкторе.

Так как это выглядит так, как currentPiece может меняться со временем, кажется, что вы используете один экземпляр FallingPiece в качестве одноэлементного контейнера для текущей падающей части. В этом случае вы должны получить вторую, третью и так далее части. Почему бы просто не сделать то же самое для первой части?

Вы говорите, что когда кусок попадает в нижнюю часть, возникает событие, которое вызывает начало новой части, чтобы начать падать. Разве вам не нужно просто запускать это событие, чтобы начать первую часть?

В целом:

Общая причина (как я вижу), что работа с зависимостями не должна выполняться в конструкторе, заключается в том, что построение объекта должно быть чисто связано с настройкой класса, чтобы он мог делать то, что ему нужно. Наличие классов на самом деле делает то, что они делают, должно выполняться посредством вызовов API-интерфейса объекта после его создания. В случае FallingPiece, когда он имеет IPieceGenerator, он имеет возможность делать все, что ему нужно. На самом деле делать это (начиная падать) должно делаться путем вызова метода в его API или (как это здесь бывает), запуская событие, на которое оно ответит.

Ответ 3

Как отмечали другие, это утверждение действительно эмпирическое правило. Это, однако, довольно сильное правило, поскольку оно дает нам более одного преимущества:

Он применяет конструктор SRP; другими словами, он сохраняет конструктор настолько чистым, насколько это возможно, потому что он делает только одно.
Это упрощает модульное тестирование, поскольку создание SUT не связано с сложным "Настройка Fixture" .
Это делает проблемы дизайна более очевидными.

Это третье преимущество, на мой взгляд, в игре здесь. Как указано, этот конструктор кричит о нарушении СРП. В чем ответственность за класс FallingPiece? Сейчас кажется, что они создают новые куски и представляют собой падающую часть.

Почему FallingPiece не является реализацией IPiece?

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

С объектно-ориентированной точки зрения, я думаю, это похоже на то, что на игровом поле должна быть коллекция экземпляров IPiece, которая знает, где они находятся и что они могут делать.

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

Ответ 4

Я бы сказал, что этот класс правильно разработан, потому что для списка параметров конструктора требуется IPieceGenerator. Поэтому это не противоречит этой идее, однако если бы это был вызов статического метода, который не содержится в списке параметров, я бы сказал, что он не отвечает этому правилу.

Ответ 5

Если вы используете инъекцию конструктора, это нормально.

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

Ответ 6

Оставляя currentPiece null, необязательно означает, что объект должен оставаться в унифицированном состоянии. Вы можете, например, преобразовать currentPiece в свойство и инициализировать его лениво при первом доступе.

Это никак не повлияет на публичный интерфейс, но сохранит конструктор легкий, что обычно хорошо.