"A method should do one thing, once and only once" - 这是什么意思？

Question

SRP: "It says that your class, or method should do only one thing"

我什么时候知道我的方法不止做一件事？
例子：我有一个 class Bus，里面有一个 List<Passenger> 和一个枚举 BusState。 Bus 的状态取决于 List<Passenger> 的大小。

public void addPassenger(Passenger p){
    this.passengerList.add(p);
    if (passengerList.size < 10)
       this.state = BusState.EMPTY;
    else if (passengerList.size < 30)
      this.state = BusState.HALF_FULL;
    else if (passengerList.size >= 30)
      this.state = BusState.FULL;
}

即使我重构这个：

public void addPassenger(Passenger p){
    this.passengerList.add(p);
    changeBusState();
}

private void changeBusState(){
    if (passengerList.size < 10)
       this.state = BusState.EMPTY;
    else if (passengerList.size < 30)
      this.state = BusState.HALF_FULL;
    else if (passengerList.size >= 30)
      this.state = BusState.FULL;
}

在我看来，方法 addPassenger() 做的不止一件事：
- 将新乘客添加到列表中
- 查看当前乘客人数
- 如有必要，更改总线状态

如何理解 SRP？这个方法不止做一件事吗？

Answer 1

我同意 addPassenger 做的不止一件事。

让它只做一件事的一种方法是删除 state 字段并使用 getState 方法 returns 基于有多少乘客的状态（假设你正在写 Java 并且 BusState 是一个枚举）：

public BusState getState() {
    if (passengerList.size < 10)
        return BusState.EMPTY;
    else if (passengerList.size < 30)
        return BusState.HALF_FULL;
    else if (passengerList.size >= 30)
        return BusState.FULL;
    else
        return BusState.UNKNOWN; // somehow the no. of passengers is negative? You can consider throwing an exception here as well...
}

Answer 2

罗伯特·马丁将 "one thing" 解释为 "one business reason to change"。对于所有代码库，没有通用的 "one" 定义，因为我们创建的 API 在不同的抽象级别上工作。因此，这完全取决于您 class 的客户是谁，以及他们可能需要进行哪些更改。

在您的情况下，可以说该方法正在做两件事：它管理总线的内容并计算状态。因此它可能有两个改变的原因：

添加乘客的不同业务逻辑：例如，可能希望验证公交车上是否有足够的位置容纳另一名乘客，如果没有则抛出异常
关于 BusState 语义的不同业务逻辑（最简单的例子：人们可能希望满载的巴士从 31 位乘客开始，而不是 30 位乘客）

在这种情况下，您可以更改 addPassenger 以专注于仅添加：

public void addPassenger(Passenger p){
    this.passengerList.add(p);
}

并更改 BusState getter 以按需执行计算（类似于 Sweeper proposed in ）：

public BusState getBusState() {
    if (passengerList.size < 10)
        return BusState.EMPTY;
    else if (passengerList.size < 30)
        return BusState.HALF_FULL;
    else if (passengerList.size >= 30)
        return BusState.FULL;
    else
        throw ...
}

"A method should do one thing, once and only once" - 这是什么意思？

"A method should do one thing, once and only once" - what does that mean?

oop

methods

coding-style