7、滥用控制语句：出现控制结构，多半是错误的提示

博主： peter
发布时间：2024 年 10 月 17 日
13 次浏览
暂无评论
3962字数
分类：坏味道方法论总结

> 循环和选择语句，可能都是坏味道。 这个坏味道就是滥用控制语句，也就是你熟悉的 if、for 等等

# 一、嵌套的代码

```cpp
public:
    void distributeEpubs(const int bookId) {
        List<Epub> epubs = this->getEpubsByBookId(bookId);
        for (Epub epub : epubs) {
            if (epub.isValid()) {
                boolean registered = this->registerIsbn(epub);
                if (registered) {
                    this->sendEpub(epub);
                }
            }                                          
        }
    }
```

我们看到了多层的缩进，for 循环一层，里面有两个 if ，又多加了两层。即便不是特别复杂的代码，也有这么多的缩进。

解决办法：

具体到这段代码，一个着手点是 for 循环，因为通常来说，for 循环处理的是一个集合，而循环里面处理的是这个集合中的一个元素。所以，我们可以把循环中的内容提取成一个函数，让这个函数只处理一个元素。

```cpp
情况1:好的写法
public:
void distributeEpubs(const int bookId) {
    List<Epub> epubs = this->getEpubsByBookId(bookId);
    for (Epub epub : epubs) {
        this->distributeEpub(epub)；
    }
}

private:
   void distributeEpub(const Epub epub) {
      if (epub.isValid()) {
          boolean registered = this->registerIsbn(epub);
          if (registered) {
              this->sendEpub(epub);
      }
   }
}
```

Note:

分解出来 distributeEpub 函数**每次只处理一个元素**。拆分出来的两个函数在缩进的问题上，就改善了一点。不要写成下面这种形式。

```cpp
情况2: 不好的写法
public:
void distributeEpubs(const int bookId) {
    List<Epub> epubs = this->getEpubsByBookId(bookId);
        this.distributeEpub(epubs)；
  
}

private:
void distributeEpub(List<Epub> epubs) {
	for (Epub epub : epubs)
	{
	    if (epub.isValid()) {
	        boolean registered = this->registerIsbn(epub);
	    if (registered) {
	        this->sendEpub(epub);
	    }
	    }
	}
}
```

条件表达式和循环常常也是提炼的信号。你可以使用`分解表达式`来处理条件表达式。**如果发现提炼出的循环很难命名，可能是因为其中做了几件事情了**。

## 1、拆分循环

> 常常能见到一些身兼数职的循环，它们一次做了两三件事情，不为别的，就因为这样可以只循环一次。一次循环中做了两件不同的事情，那么每次修改循环都要理解这两件事情。

情况1: 如果一个循环只计算一个值，那么它直接返回该值即可；

情况2: 但如果循环做了太多件事情，那就只得返回结构型数据或者通过局部变量传值了；

Tips:

如果将情况2重构为情况1这种情况时，需要考虑是否会成为性能瓶颈。

举一个粒子，比如一个循环里面有两件事，一个是计算收入总值（数据累加），一个是找出最小收入

```cpp
int total = 0;
vector<int> datas = [15, 25, 3, 65, 15];
int min = data[0];
for(auto data : datas)
{
    total += data[i];
    if (data[i] < min) 
    {
        min = data[i];
    }
}
```

解决办法：

使用情况2这种方式进行拆分，但是需要删除涉及到重复计算的逻辑，否则会累加出错。

第一步：拆分

```cpp
int total = 0;
vector<int> datas = [15, 25, 3, 65, 15];
for(auto data : datas)
{
    total += data[i];
}

vector<int> datas = [15, 25, 3, 65, 15];
int min = data[0];
for(auto data : datas)
{
    if (data[i] < min) 
    {
        min = data[i];
    }
}
```

第二步：封装

```cpp
int calTotalData(vector<int> datas)
{
		int total = 0;
    for(auto data : datas)
		{
		    total += data[i];
		}
		return total;
}

int findMinDate(vector<int> datas)
{
		int min = data[0];
		for(auto data : datas)
		{
		    if (data[i] < min) 
		    {
		        min = data[i];
		    }
		}
	return min;
}

```

在做提炼之前，最好先用移动语句微调一下代码顺序，将与循环相关的变量先搬移到一起。

针对这种for语句的循环其实也可以利用STL中自带的算法库来优化。比如for\_each 、find\_if、`copy[参见effective STL 第五条]`等

## 2、if 和 else

> `else 也是一种坏味道，这是挑战很多程序员认知的` 通常来说，if 语句造成的缩进，很多时候都是在检查某个先决条件，只有条件通过时，才继续执行后续的代码。这样的代码可以使用卫语句（guard clause）来解决，也就是设置单独的检查条件，不满足这个检查条件时，立刻从函数中返回。

`一种典型的重构手法：以卫语句取代嵌套的条件表达式（Replace Nested Conditional with Guard Clauses）。`

针对上面的情况1进行修改：

```cpp

private：
void distributeEpub(const Epub epub) {
    if (!epub.isValid()) {
        return;
    }
  
    boolean registered = this->registerIsbn(epub);
    if (!registered) {
        return;
    }
  
    this->sendEpub(epub);
}
```

**有一个衡量代码复杂度常用的标准，叫做[圈复杂度](https://en.wikipedia.org/wiki/Cyclomatic_complexity)（Cyclomatic complexity，简称 CC），圈复杂度越高，代码越复杂，理解和维护的成本就越高**。

**我们也可以利用 Checkstyle 就可以做圈[复杂度的检查](https://checkstyle.sourceforge.io/config_metrics.html#CyclomaticComplexity)，你可以限制最大的圈复杂度。**

Note: 可以在else if中直接使用lamber表达式

```cpp
void HwaBbsObsaiLinkSync::onHandlePortPipeResp(portPipeRespType& respMsg,
    const std::string& message)
{
    if (respMsg.portPipeStatuses.empty())
    {
        log_ << error << __FUNCTION__ << "Received " << message << " with empty portPipeStatuses. ";

handleTaskResultCallback(ETaskResult::HwapiError);
    }
    else if (std::all_of(respMsg.portPipeStatuses.begin(), respMsg.portPipeStatuses.end(),
        [](const SBbbPortPipeStatus& portPipeStatus)
        { return EStatus_NoError == portPipeStatus.status; }))
```

## 3、重复的 Switch

> 这也是一种典型的坏味道：重复的 switch（Repeated Switch）。 **如果有多个switch语句基于同一个条件进行分支选择**，就应该使用`以多态取代条件表达式`

```cpp
public：
    double getBookPrice(const User user, const Book book) {
    double price = book.getPrice();
    switch (user.getLevel()) {
        case UserLevel.SILVER:
        return price * 0.9;
        case UserLevel.GOLD: 
        return price * 0.8;
        case UserLevel.PLATINUM:
        return price * 0.75;
        default:
        return price;
    }
}

public:
    double getEpubPrice(final User user, final Epub epub) {
    double price = epub.getPrice();
    switch (user.getLevel()) {
        case UserLevel.SILVER:
        return price * 0.95;
        case UserLevel.GOLD: 
        return price * 0.85;
        case UserLevel.PLATINUM:
        return price * 0.8;
        default:
        return price;
    }
}
```

这两段代码，分别计算了用户在网站上购买作品在线阅读所支付的价格，以及购买 EPUB 格式电子书所支付的价格。其中，用户实际支付的价格会根据用户在系统中的用户级别有所差异，级别越高，折扣就越高。

这两个函数里出现了类似的代码，其中最类似的部分就是 switch，都是根据用户级别进行判断。

解决办法：

之所以会出现**重复的 switch**，通常都是缺少了一个模型。所以，应对这种坏味道，`重构的手法是：以多态取代条件表达式（Relace Conditional with Polymorphism）`

```cpp
class IUserLevel {
    double getBookPrice(Book book);
    double getEpubPrice(Epub epub);
}

class RegularUserLevel : IUserLevel {
public:
    double getBookPrice(const Book book) {
        return book.getPrice();
    }
  
    double getEpubPrice(const Epub epub) {
        return epub.getPrice();
    }
}

class GoldUserLevel : IUserLevel {
    public:
    double getBookPrice(const Book book) {
        return book.getPrice() * 0.8;
    }

double getEpubPrice(const Epub epub) {
        return epub.getPrice() * 0.85;
    }
}

class SilverUserLevel : IUserLevel {
    public:
    double getBookPrice(const Book book) {
        return book.getPrice() * 0.9;
    }
  
    double getEpubPrice(const Epub epub) {
        return epub.getPrice() * 0.85;
    }
}

class PlatinumUserLevel : IUserLevel {
    public:
    double getBookPrice(const Book book) {
        return book.getPrice() * 0.75;
    }
  
    double getEpubPrice(const Epub epub) {
        return epub.getPrice() * 0.8;
    }
}

错误的重构：getBookPrice的形参不一定会变
public:
double getBookPrice(const IUserLevel user, const Book book) {
    return user.getBookPrice(book);
}

正确的重构：需要再封装一个函数将user 转化为 UserLevel 
public:
double getBookPrice(const User user, const Book book) {
    IUserLevel level = user.getUserLevel();
    return level.getBookPrice(book);
}

```

调用的地方：

```cpp
public:
double getBookPrice(const User user, const Book book) {
    IUserLevel level = user.getUserLevel();
    return level.getBookPrice(book);
}

public:
double getEpubPrice(cosnt User user, const Epub epub) {
    IUserLevel level = user.getUserLevel()
    return level.getEpubPrice(epub);
}

IUserLevel User::getUserLevel()// 个人感觉这个地方叫createUserLevel更合适
{
	  switch (user.getLevel()) 
    {
    case UserLevel.SILVER:
        return new SilverUserLevel();
    case UserLevel.GOLD: 
        return new GoldUserLevel();
    case UserLevel.PLATINUM:
        return new PlatinumUserLevel();
    default:
        return new RegularUserLevel();
    }
}
```

我们看到的是一连串的“ if..else”。我们都知道，switch 其实就是一堆“ if..else” 的简化写法，二者是等价的，所以，这个重构手法，以多态取代的是条件表达式，而不仅仅是取代 switch。

**这个例子其实使用的就是策略模式，如果不方便叫UserLevel 这个名字的话，可以使用**

**名字UserLevel Strategy来命名**。

`Tips: 并不是所有的if/else都需要使用多态来替代，但是当满足如下两种情况中的任意一种时，建议使用多态来重构`。

情况1：

最明显的征兆就是有好几个函数都有基于类型代码的switch语句，若真如此，我就可以针对switch语句中的每种分支逻辑创建一个类，用多态来承载各个类型特有的行为，从而去除重复的分支逻辑。

情况2：

基础逻辑可能是最常用的，也可能是最简单的。我可以把基础逻辑放到父类，把变体逻辑放到子类中。

最后修改：2025 年 07 月 01 日

如果觉得我的文章对你有用，请随意赞赏

发表评论取消回复
使用cookie技术保留您的个人信息以便您下次快速评论，继续评论表示您已同意该条款

评论 *

私密评论

名称 *

🎲

邮箱 *

地址

7、滥用控制语句：出现控制结构，多半是错误的提示

peter • 2024 年 10 月 17 日

> 循环和选择语句，可能都是坏味道。 这个坏味道就是滥用控制语句，也就是你熟悉的 if、for 等等

# 一、嵌套的代码

我们看到了多层的缩进，for 循环一层，里面有两个 if ，又多加了两层。即便不是特别复杂的代码，也有这么多的缩进。

解决办法：

Note:

分解出来 distributeEpub 函数**每次只处理一个元素**。拆分出来的两个函数在缩进的问题上，就改善了一点。不要写成下面这种形式。

```cpp
情况2: 不好的写法
public:
void distributeEpubs(const int bookId) {
    List<Epub> epubs = this->getEpubsByBookId(bookId);
        this.distributeEpub(epubs)；
  
}

## 1、拆分循环

情况1: 如果一个循环只计算一个值，那么它直接返回该值即可；

情况2: 但如果循环做了太多件事情，那就只得返回结构型数据或者通过局部变量传值了；

Tips:

如果将情况2重构为情况1这种情况时，需要考虑是否会成为性能瓶颈。

举一个粒子，比如一个循环里面有两件事，一个是计算收入总值（数据累加），一个是找出最小收入

```cpp
int total = 0;
vector<int> datas = [15, 25, 3, 65, 15];
int min = data[0];
for(auto data : datas)
{
    total += data[i];
    if (data[i] < min) 
    {
        min = data[i];
    }
}
```

解决办法：

使用情况2这种方式进行拆分，但是需要删除涉及到重复计算的逻辑，否则会累加出错。

第一步：拆分

```cpp
int total = 0;
vector<int> datas = [15, 25, 3, 65, 15];
for(auto data : datas)
{
    total += data[i];
}

vector<int> datas = [15, 25, 3, 65, 15];
int min = data[0];
for(auto data : datas)
{
    if (data[i] < min) 
    {
        min = data[i];
    }
}
```

第二步：封装

```cpp
int calTotalData(vector<int> datas)
{
		int total = 0;
    for(auto data : datas)
		{
		    total += data[i];
		}
		return total;
}

int findMinDate(vector<int> datas)
{
		int min = data[0];
		for(auto data : datas)
		{
		    if (data[i] < min) 
		    {
		        min = data[i];
		    }
		}
	return min;
}

```

在做提炼之前，最好先用移动语句微调一下代码顺序，将与循环相关的变量先搬移到一起。

针对这种for语句的循环其实也可以利用STL中自带的算法库来优化。比如for\_each 、find\_if、`copy[参见effective STL 第五条]`等

## 2、if 和 else

`一种典型的重构手法：以卫语句取代嵌套的条件表达式（Replace Nested Conditional with Guard Clauses）。`

针对上面的情况1进行修改：

```cpp

**我们也可以利用 Checkstyle 就可以做圈[复杂度的检查](https://checkstyle.sourceforge.io/config_metrics.html#CyclomaticComplexity)，你可以限制最大的圈复杂度。**

Note: 可以在else if中直接使用lamber表达式

## 3、重复的 Switch

> 这也是一种典型的坏味道：重复的 switch（Repeated Switch）。 **如果有多个switch语句基于同一个条件进行分支选择**，就应该使用`以多态取代条件表达式`

这两个函数里出现了类似的代码，其中最类似的部分就是 switch，都是根据用户级别进行判断。

解决办法：

```cpp
class IUserLevel {
    double getBookPrice(Book book);
    double getEpubPrice(Epub epub);
}

class GoldUserLevel : IUserLevel {
    public:
    double getBookPrice(const Book book) {
        return book.getPrice() * 0.8;
    }

double getEpubPrice(const Epub epub) {
        return epub.getPrice() * 0.85;
    }
}

错误的重构：getBookPrice的形参不一定会变
public:
double getBookPrice(const IUserLevel user, const Book book) {
    return user.getBookPrice(book);
}

```

调用的地方：

```cpp
public:
double getBookPrice(const User user, const Book book) {
    IUserLevel level = user.getUserLevel();
    return level.getBookPrice(book);
}

public:
double getEpubPrice(cosnt User user, const Epub epub) {
    IUserLevel level = user.getUserLevel()
    return level.getEpubPrice(epub);
}

**这个例子其实使用的就是策略模式，如果不方便叫UserLevel 这个名字的话，可以使用**

**名字UserLevel Strategy来命名**。

`Tips: 并不是所有的if/else都需要使用多态来替代，但是当满足如下两种情况中的任意一种时，建议使用多态来重构`。

情况1：

情况2：

基础逻辑可能是最常用的，也可能是最简单的。我可以把基础逻辑放到父类，把变体逻辑放到子类中。

发表评论 取消回复 使用cookie技术保留您的个人信息以便您下次快速评论，继续评论表示您已同意该条款

7、 滥用控制语句：出现控制结构，多半是错误的提示

发表评论取消回复
使用cookie技术保留您的个人信息以便您下次快速评论，继续评论表示您已同意该条款

7、滥用控制语句：出现控制结构，多半是错误的提示