C Programming: how to avoid code duplication without losing clarity

Question

edit: Thanks to all repliers. I should have mentioned in my original post that I am not allowed to change any of the specifications of these functions, so solutions using assertions and/or allowing to dereference NULL are out of the question. With this in mind, I gather that it's either I go with a function pointer, or just leave the duplication as it is. For the sake of clarity I'd like to avoid function pointers this time.

original: I am trying to avoid code duplication without losing clarity. often when working on a specific assignment (Uni - undergrad) I recognize these patterns of functions return , but not always with a "great-job" solution..

What would any of you suggest I should do (pointers to functions, macros, etc.) with these three C functions that check some of their arguments in the same way to make the checking more modular (it should be more modular, right?)?

BTW these are taken directly from a HW assignment, so the details of their functionality are not concerning my question, only the arguments checking at the function's top.

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

Any advice on how (specifically) to avoid the code duplication would be appreciated.

Thanks for your time! :)

Answer 1

I would create a function to check the team object:

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

And then reference that + your other checks at the top of each function, for example

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

Otherwise, if each function is different then leave the checks as different!