调用其他代码是否违反(SOLID)单一职责原则(SRP)?

Question

考虑到这个带有业务逻辑的类：

public static class OrderShipper
{
    public static void ShipOrder(Order order) {
        AuthorizationHelper.AuthorizedUser();

        using (new PerformanceProfiler()) {
            OperationRetryHelper.HandleWithRetries(() => ShipOrderInTransaction(order));
        }
    }

    private static void ShipOrderInTransaction(Order order) {
        using (var transaction = new TransactionHelper()) {
            ShipOrderInternal(order);

            transaction.Commit();
        }            
    }

    private static void ShipOrderInternal(order) {
        // lots of business logic
    }
}

该类包含一些业务逻辑，并执行一些横切关注点。虽然毫无疑问，这个类违反了开放/封闭原则，但是这个类是否违反了单一职责原则？ 我有疑虑，因为这个类本身不负责授权用户、分析性能和处理事务。

毫无疑问，这是一个糟糕的设计，因为该类仍然（静态地）依赖于这些横切关注点，但仍然存在疑问：它是否违反了SRP？如果是，为什么？

- user782596

---

如果你把它的职责设置为“发货订单”，我会说“不”。 - Sasha

Answer 1

这是一个好问题，标题有点误导（如果没有“调用其他代码”，你不太可能构建应用程序）。请记住，SOLID原则更多的是指导方针，而不是必须遵循的绝对规则；如果您将SRP推向其逻辑结论，您将最终每个类只有一个方法。减少横切关注的影响的方法是创建一个尽可能易于使用的外观。在您的示例中，您做得很好-每个横切关注只使用一行。

另一种实现这一点的方法是通过AOP，在C#中可以使用PostSharp或通过IoC拦截。

- Alex

---

不要忘记，您也可以使用好用的装饰器来添加横切关注点（正如这篇文章所示）。 - Steven

MikeSW在他的回答中表示，我的代码确实违反了SRP原则。你对他的观点有什么看法？ - user782596

这取决于你的看法。我认为原则可能被过分强调了 - 在我看来，你没有违反SRP，因为你类中的根本变化原因是发货业务逻辑。其他职责已经委托给其他地方。我唯一能建议改进的地方是使用工厂来创建你的Profiler和Transaction对象 - 这样你只需要绑定外部依赖的接口，这是可以接受的（假设接口不会改变）。 - Alex

Answer 2

通过将类转换为命令，使其更加易于识别:

// Command pattern
public class ShipOrder
{
    ITransactionFactory _transactionFactory;


    public OrderShipper(ITransactionFactory factory)
    {
        if (factory == null) throw new ArgumentNullException("factory");

        _transactionFactory = factory;
    }

    [PrincipalPermission(Roles = "User")]
    public void Execute(Order order) 
    {
        if (order == null) throw new ArgumentNullException("order");

        using (new PerformanceProfiler()) 
        {
            HandleWithRetries(() => ShipOrderInTransaction(order));
        }
    }

    private void ShipOrderInTransaction(Order order) 
    {
        using (var transaction = _transactionFactory.Create()) 
        {
            ShipOrderInternal(order);

            transaction.Commit();
        }            
    }

    protected void ShipOrderInternal(order) 
    {
        // bussiness logic which is divided into different protected methods.
    }
}

因此，您可以使用以下方式调用它：

var cmd = new ShipOrder(transactionFactory);
cmd.Execute(order);

那相当稳定。

- jgauffin

---

你没有回答我的问题。我的代码是否违反了SRP原则？ - user782596

那是相当稳固的，这就是我的回答。也就是说，你的代码很可靠（只需要一些改进）。 - jgauffin

我不明白OP代码如何符合SOLID原则。你的解决方案可能类似，但对我来说，它显然是与OP的方法不同的方法。 - MikeSW

我认为我的代码不符合SOLID原则。它违反了OCP原则，因为每次添加新的横切关注点时，都必须更改代码。但是如果我理解正确，你认为它没有违反SRP原则。 - user782596

你说得对。我的代码很稳定，但OP的代码只是SRP。OCP/LSP与静态类不太兼容。 - jgauffin

此外，OP 的代码也违反了 DIP 原则，而您的代码也解决了这个问题。 - Steven

Answer 3

在一个类方法协调其他类的活动是没有问题的，这并不会违反SRP。如果那些类的逻辑是 OrderShipper 的一部分，那么你才会破坏它。

我不确定 PerformanceProfiler 做了什么，但它是那里唯一看起来奇怪的组件。

- Claudio Redi

---

MikeSW声称这段代码实际上违反了SRP原则。你对他的说法有何看法？ - user782596

Answer 4

是的，至少根据类名，它违反了SRP原则。

该类本身不负责授权用户、性能分析和处理交易。

你自己回答了，它应该仅包含发货订单逻辑。而且它不应该是静态的（为什么是静态的？！）。

@jgauffin提供的解决方案是一种可能性，尽管我并不完全相信OrderShipper应该知道事务还是只是其中的一部分。在我看来，性能分析器在这个类中没有任何位置。但只有这些信息，我无法建议一个解决方案。性能分析虽然是一个横切关注点，但最好在这个类之外处理，也许可以使用属性。

顺便说一句，使用消息驱动方法（如jgauffin所建议的），它应该允许基础架构提供性能分析和可靠性（HandleWithRetries）支持。

- MikeSW

---

你是唯一一个声称我的代码实际上违反了SRP的人。你能用一些证据来支持吗？也许引用定义或其他参考资料？ - user782596

笑，定义和参考文献...这个类应该做什么？发货订单吗？那就是它的责任。如果它除了处理发货订单之外还处理其他问题，那么你就有不止一个单一的责任。例如，这个类永远不应该处理用户授权。关于分析，我没有足够的信息，但我非常确定你使用静态类和静态方法会有很大的影响。 - MikeSW