代码之家 › 专栏 › 技术社区 › Mapi

“一种方法应该做一件事,一次而且只能做一次”-这是什么意思?

coding-style methods oop

Mapi · 技术社区 · 6 年前

SRP:“它说您的类或方法应该只做一件事”

我什么时候知道我的方法是做不止一件事?
示例: 我有课 Bus 使用 List<Passenger> 和枚举 BusState 在里面。的状态 公共汽车 取决于 列表(<);乘客> 。

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?这种方法做的不止一件事吗?

2 回复 | 直到 6 年前

Sweeper 6 年前

我同意 addPassenger 正在做不止一件事。

让它只做一件事的一种方法是删除 state 字段并具有 getState 方法,该方法返回基于乘客数量的状态(假设您使用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...
}

BartoszKP 6 年前

罗伯特·马丁将“一件事”解释为“改变的一个商业理由”。对于所有的代码库,没有“一”的通用定义,因为我们创建的API在不同的抽象级别上工作。因此,这完全取决于谁是您的类的客户机,以及他们可能需要进行哪些更改。

在您的案例中,可以说该方法正在做两件事:它管理总线的内容并计算状态。因此,有两个原因可以改变:

添加乘客的不同业务逻辑:例如,您可能希望验证总线中是否有足够的位置容纳另一名乘客,如果没有,则抛出异常
关于语义的不同业务逻辑 BusState (最简单的例子:有人可能希望全车从31名乘客开始,而不是30名乘客)

在这种情况下,你可以改变 addPassenger 仅关注添加:

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

并更改 总线状态 getter按需执行计算(类似于 Sweeper 建议于 their answer ):

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 ...
}