Является ли это запахом кода для инъекции зависимостей и устанавливает один из его членов в `this`?

Является ли это запахом кода для инъекции зависимостей и установки одного из его свойств в ваш текущий экземпляр? Я установил свой код таким образом, чтобы полностью изолировать реализацию сервиса. У меня есть серия тестов, которые все проходят (включая установку экземпляра StreamingSubscriber в логическом классе).

Например

public class StreamingSubscriber
{
    private readonly ILogic _logic;

    public StreamingSubscriber(ILogic logic)
    {            
        _logic = logic;

        // Not sure I like this...
        _logic.StreamingSubscriber = this;
    }

    public void OnNotificationEvent(object sender, NotificationEventArgs args)
    {
        // Do something with _logic
        var email = _logic.FetchEmail(args);
        // consume the email (omitted for brevity)
    }
}

public class ExchangeLogic : ILogic
{   
    public StreamingSubscriber StreamingSubscriber { get; set; }

    public void Subscribe()
    {
        // Here is where I use StreamingSubscriber
        streamingConnection.OnNotificationEvent += StreamingSubscriber.OnNotificationEvent;
    }

    public IEmail FetchEmail(NotificationEventArgs notificationEventArgs)
    {
        // Fetch email from Exchange
    }
}

Если это запах кода, как вы его исправите?

Изменить

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

    [Test]
    public void FiringOnNotificationEvent_WillConsumeEmail()
    {
        // Arrange
        var subscriber = new StreamingSubscriber(ConsumerMock.Object, ExchangeLogicMock.Object);

        // Act
        subscriber.OnNotificationEvent(It.IsAny<object>(), It.IsAny<NotificationEventArgs>());

        // Assert
        ConsumerMock.Verify(x => x.Consume(It.IsAny<IEmail>()), Times.Once());
    }

Теперь это, очевидно, невозможно достичь без проведения полномасштабных интеграционных тестов. Если я сказал, что мой ExchangeLogic должен использовать электронную почту.

Ответ 1

Это не поражает меня как запах кода как таковой, нет.

Однако, если эта работа через сеттер создает ситуацию, когда у вас может быть проблема с синхронизацией - что делать, если кто-то звонит, а StreamingSubscriber еще не установлен? Теперь вам нужно написать код, чтобы защититься от этого. Я бы избегал использовать setter и переставлял его так, чтобы вы назовете "_logic.Subscribe(this)".

Ответ 2

Да, это плохо; вы создаете циклическую зависимость.

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

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

В книге "Интенсивность зависимостей" в .NET обсуждается это в главе 6: реорганизация DI, раздел 6.3: разрешение циклических зависимостей.

Ответ 3

Я не вижу, чтобы этот сценарий был слишком вонючим. Это вполне законный случай, чтобы иметь круглую ссылку между компонентом и его зависимостью. Вы можете сделать это на 100% пуленепробиваемым, введя factory, это зависит от вас, чтобы судить, есть ли в этом польза.

public class StreamingSubscriber
{
    private readonly ILogic _logic;

    public StreamingSubscriber(ILogicFactory logicFactory)
    {            
        _logic = logicFactory.Create(this);
    }

    public void OnNotificationEvent(object sender, NotificationEventArgs args)
    {
        // Do something with _logic
        var email = _logic.FetchEmail(args);
        // consume the email (omitted for brevity)
    }
}

public class ExchangeLogic : ILogic
{   
    private readonly StreamingSubscriber _StreamingSubscriber;

    public ExchangeLogic (StreamingSubscriber subscriber){
       _StreamingSubscriber = streamingSubscriber;
       Subscribe();
    }

    private void Subscribe()
    {
        // Here is where I use StreamingSubscriber
        streamingConnection.OnNotificationEvent += _StreamingSubscriber.OnNotificationEvent;
    }

    public IEmail FetchEmail(NotificationEventArgs notificationEventArgs)
    {
        // Fetch email from Exchange
    }
}

Я нахожу тот факт, что ваша логическая реализация провоцирует событие непосредственно на его метод зависимостей, более тревожный, чем вся круговая справочная проблема. Я бы выделил это так, чтобы изменения в StreamingConnection не влияли на StreamingSubscriber, вы можете сделать это с помощью простого анонимного метода, подобного этому (вы также можете удалить sender из подписи, если хотите, половину времени, когда я мне это не нужно):

streamingConnection.OnNotificationEvent += (sender, args) => _StreamingSubscriber.OnNotificationEvent(sender, args);