较大的函数与重复相同代码的较小函数

Larger Function vs Smaller Functions that Repeat Same Code

本文关键字：函数 代码      更新时间：2023-10-16

我一直在阅读有关保持函数简单且仅针对一个目的的信息。而且即使是进行简单计算并打印出结果的函数也已经太多了。

我一直在为一个小游戏开发一个物品商店系统。商店拥有游戏中所有可用物品的矢量。每个项目都会跟踪自己的计数(这很糟糕吗？(，如果其计数为零，则不会显示在库存输出中。

当玩家(

骑手(想要购买物品时，商店应该检查玩家是否有足够的信用额度，以及该物品是否有库存，然后再从商店中移除该物品(然后添加到玩家的物品栏中(。

我将所有这些组合到一个函数中，认为这样可以避免重复遍历 Items 的向量。

/* ItemShop.cpp
 * non-binary boolean:
 *   returns 0 if item is not in stock
 *   returns 1 if rider has insufficient creds to buy the item
 *   returns 2 if purchase is successful */
int ItemShop::buyItem(const string& itemName, const int cred, Item& newItem) {
    // go through shop inventory and search for item with given name
    for (int i = 0; i < items.size(); i++) {
        if (items[i].getName() == itemName) {
            if (items[i].getCount() > 0) { // item is in stock
                if (items[i].getValue() <= cred) { // rider has enough creds to buy the item
                    newItem = items[i];    // pass a copy of the item by ref
                    newItem.setCount(1);   // take one of that item
                    items[i].removeOne();  // remove one from the shop
                    return 2; // purchase is successful
                }
                return 1; // rider cannot afford to buy the item
            }
        }
    }
    return 0; // item is not in stock
}

在执行这种方法时，我最终得到了一个更大的多用途函数，但我不必多次遍历 Item 向量。我想如果我将函数分解为单独的函数，它们将是：

  1. 检查商品是否有库存
  2. 检查玩家是否负担得起
  3. 交易
这些函数中

的每一个都必须遍历并在向量中找到该项(除非我从函数中传递它的引用？

总而言之，更少的代码重复是否使我的方法合理？如果没有，我应该如何分解它？

两个建议：

  1. 将项目存储在字符串所在的std::map<string,Item>中项名称。这将删除搜索循环。
  2. 使用枚举而不是 int 作为返回值。

你也可以为不同的检查实现简单的函数，但我会说这是一个品味问题。

您可以将其分解为以下操作：

  • 按名称查找相应的项(这是循环的主要作用(，并返回对找到的项的引用
  • 检查库存是否有足够的商品。 item本身已经携带了这些信息，但它不需要在循环中执行。
  • 检查玩家是否负担得起。显然，您已经有了cred值，它不需要在循环中执行
  • 如果满足后两个条件，则执行事务，则无需在循环内执行

你的函数没有做太多事情 - 你正在购买一个项目，其中包括这三个必要的步骤(顺便说一句：你没有错过付款吗？

但是(除了其他可以改进的地方(，您应该确保这些步骤不会混合在一起。特别是尽可能避免嵌套结构。
例如，您可以像这样重写您的身体(无需更改界面(：

int ItemShop::buyItem(const string& itemName, const int cred, Item& newItem) {
    //find item
    auto it = std::find_if(items.begin(), items.end(), [&](const Item& item) {return item.getName() == itemName; });
    //Check if Item can be purchased
    if (it == items.end() || it->getCount == 0) {
        return 0; //item is not in stock
    }   
    if (it->getValue() > cred) {
        return 1; //rider can't afford item
    }
    //buy item
    newItem = *it;          // pass a copy of the item by ref <- Avoid that if possible
    newItem.setCount(1);    // take one of that item
    it->removeOne();        // remove one from the shop
    return 2; // purchase is successful 
}

我确实认为一个项目保持自己的计数是一个糟糕的主意。"库存"(或该订单上的东西(跟踪库存中的项目以及每个项目的数量。一个项目应该就是：一个项目。

我认为使用 [unordered_] 地图的建议是一个很好的建议。它预先实现了当前函数中最复杂的部分(尽管没有一个是特别复杂的(。

请注意，当/如果涉及多个线程时，其中一些可能会变得更加棘手。如果涉及多个执行线程，您当前的"如果我们能做到这一点，那就去做"的模式就会崩溃，因为它引入了竞争条件。您需要确保从库存中删除物料并支付物料作为单个原子交易进行。只要您确定它只涉及一个执行线程，您当前的方法就是安全的。

相关文章：