C 编程：如何避免代码重复而不失去清晰度

Question

编辑：感谢所有回复者。我应该在我原来的帖子中提到，我不允许更改这些函数的任何规范，因此使用断言和/或允许取消引用 NULL 的解决方案是不可能的。 考虑到这一点，我认为要么我使用函数指针，要么直接保留重复项。为了清楚起见，这次我想避免使用函数指针。

原文： 我试图避免代码重复而不失去清晰度。 通常在处理特定任务（大学本科）时，我认识到这些函数模式返回，但并不总是有一个“出色的工作”解决方案..

你们中的任何人建议我应该用这三个 C 函数做什么（指向函数、宏等）以相同的方式检查其参数的 一些，以使检查更加模块化（它应该更加模块化，对吗？）？

顺便说一句，这些是直接从硬件分配中获取的，因此它们的功能细节与我的问题无关，只是在函数顶部检查的参数。

 teamIsDuplicateCoachName(Team team, bool* isDuplicate) {
    TeamResult result = TEAM_SUCCESS;
    if (!team || !isDuplicate) {
        result = TEAM_NULL_ARGUMENT;
    } else if (teamEmpty(team)) {
        result = TEAM_IS_EMPTY;
    } else {
        for (int i = 0; i < team->currentFormations; ++i) {
            if (teamIsPlayerInFormation(team->formations[i], team->coach)) {
                *isDuplicate = true;
                break;
            }
        }
    }
    return result;
}

TeamResult teamGetWinRate(Team team, double* winRate) {
    TeamResult result = TEAM_SUCCESS;
    if (!team || !winRate) {
        result = TEAM_NULL_ARGUMENT;
    } else {
        int wins = 0, games = 0;
        for (int i = 0; i < team->currentFormations; ++i) {
            Formation formation = team->formations[i];
            if (formationIsComplete(formation)) {
                games += formation->timesPlayed;
                wins += formation->timesWon;
            }
        }
        double win = ( games == 0 ) ? 0 : (double) wins / games;
        assert(win >= 0 && win <= 1);
        *winRate = win;
    }
    return result;
}



TeamResult teamGetNextIncompleteFormation(Team team, Formation* formation,
        int* index) {
    TeamResult result = TEAM_SUCCESS;
    if (!team || !formation || !index) {
        result = TEAM_NULL_ARGUMENT;
    } else {
        *formation = NULL; /* default result, will be returned if there are no incomplete formations */
        for (int i = 0; i < team->currentFormations; ++i) {
            Formation formationPtr = team->formations[i];
            if (!formationIsComplete(formationPtr)) {
                *formation = formationPtr;
                *index = i;
                break;
            }
        }
    }
    return result;
}

任何关于如何（特别是）避免代码重复的建议将不胜感激。

感谢您的宝贵时间！ :)

Answer 1

将空值传递给这些函数似乎是一个编码错误。处理这种情况主要有三种方法。

1. 处理错误的空值并返回错误值。这引入了检查参数以返回错误值的额外代码，以及每个调用站点周围的额外代码，现在必须处理错误返回值。可能这些代码都没有经过测试，因为如果您知道代码错误地传递了空值，您只需修复它。
2. 使用 assert 检查参数的有效性，产生干净的错误消息，清楚地阅读前提条件，但有一些额外的代码。
3. 没有前置条件检查，并在您延迟 NULL 时调试段错误。

根据我的经验，3 通常是最好的方法。它添加了零额外代码，并且段错误通常与您从 2 中获得的干净错误消息一样容易调试。但是，您会发现许多软件工程师更喜欢 2，这是一个品味问题。

您的代码（模式 1）有一些明显的缺点。首先，它添加了无法优化的额外代码。其次，更多的代码意味着更多的复杂性。第三，不清楚这些函数是否应该能够接受损坏的参数，或者代码是否只是在出现问题时帮助调试。

Answer 2

我会创建一个函数来检查团队对象：

TeamResult TeamPtrCheck(Team *team)
{
    if (team == NULL)
        return TEAM_NULL_ARGUMENT;
    else if (teamEmpty(team))
        return TEAM_IS_EMPTY;
    else
        return TEAM_SUCCESS;
}

然后在每个函数的顶部引用那个 + 你的其他检查，例如

TeamResult = TeamPtrCheck(team);
if (TeamResult != TEAM_SUCCESS)
    return TeamResult;
if (winRate == NULL)
    return TEAM_NULL_ARGUMENT;

否则，如果每个函数 不同，则将检查保留为不同！

Answer 3

如果您担心每个函数开始时 NULL 检查的重复，我不会。它让用户清楚地知道，您只是在进行任何工作之前进行输入验证。不用担心那几行。

一般来说，不要为这样的小事出汗。

Answer 4

有一些技术可以减少您感知的冗余，其中一种是否适用在很大程度上取决于您正在检查的条件的性质。无论如何，我建议不要使用任何（预处理器）技巧来减少隐藏实际发生的事情的重复。

如果您有不应该发生的情况，检查它的一种简洁方法是使用断言。用一个断言你基本上说：这个条件必须是真的，否则我的代码有一个错误，请检查我的假设是否正确，如果不是，请立即终止我的程序。这通常是这样使用的：

#include <assert.h>

void foo(int a, int b) {
    assert((a < b) && "some error message that should appear when the assert fails (a failing assert prints its argument)");
    //do some sensible stuff assuming a is really smaller than b
}

一个特殊情况是指针是否为空的问题。做类似的事情

void foo(int* bar) {
    assert(bar);
    *bar = 3;
}

毫无意义，因为取消引用空指针会在任何健全的平台上安全地对您的程序进行分段错误，因此以下内容将同样安全地停止您的程序：

void foo(int* bar) {
    *bar = 3;
}

语言律师可能对我所说的不满意，因为根据标准，取消引用空指针是未定义的行为，从技术上讲，编译器将被允许生成格式化硬盘的代码。但是，取消引用空指针是一个非常常见的错误，您可以期望您的编译器不会用它做愚蠢的事情，并且您可以期望您的系统特别注意确保如果您尝试这样做硬件会尖叫。这种硬件检查是免费的，断言需要几个周期来检查。

然而，断言（和段错误空指针）仅适用于检查致命条件。如果您只是检查使函数内部的任何进一步工作毫无意义的条件，我会毫不犹豫地使用提前返回。它通常更具可读性，特别是因为语法突出显示很容易向读者揭示返回语句：

void foo(int a, int b) {
    if(a >= b) return;
    //do something sensible assuming a < b
}

使用此范例，您的第一个函数将如下所示：

TeamResult teamIsDuplicateCoachName(Team team, bool* isDuplicate) {
    if(!team || !isDuplicate) return TEAM_NULL_ARGUMENT;
    if(teamEmpty(team)) return TEAM_IS_EMPTY;

    for (int i = 0; i < team->currentFormations; ++i) {
        if (teamIsPlayerInFormation(team->formations[i], team->coach)) {
            *isDuplicate = true;
            break;
        }
    }
    return TEAM_SUCCESS;
}

我相信，这比带有 if 的版本更清晰简洁。

Answer 5

这或多或少是一个设计问题。如果上面的函数都是静态函数（或者只有一个是外部函数），那么整个“函数包”应该检查条件 - 执行流程 - 为每个对象一次，并让低级函数的实现细节假设输入数据有效。

例如，如果您回到创建、分配和初始化team 的地方，以及创建、分配和初始化formation 的地方，并构建规则那里团队存在并且不存在重复项，您不必验证输入，因为根据定义/构造，它将始终存在。这是前置条件的例子。不变量将是这些定义的真实性的持久性（没有函数可以在返回时改变不变量状态），而后置条件则相反（例如，当它们是 free'd 但指针仍然存在于某处时）。

话虽如此，在 C 中操作“类对象”数据，我个人的偏好是创建创建、返回和销毁此类对象的外部函数。如果成员在具有最小 .h 接口的 .c 文件中保持静态，则在概念上与面向对象编程相似（尽管您永远无法使成员完全“私有”）。

Answer 6

感谢所有回复者。我应该在我的原始帖子中提到我不允许更改这些函数的任何规范，因此使用断言和/或允许取消引用 NULL 的解决方案是不可能的，尽管我'其他场合会考虑的。

考虑到这一点，我认为要么我使用函数指针，要么将重复项保持原样。为了清楚起见，这次我想避免使用函数指针。