C initialize reference and assign variable together-CodePudding

I have this section in my code where I am using an if else, and the ternary operator on the same bool condition. Is there a more elegant way to do this?

bool UseGroups //input parameter to a function.

    std::vector<std::vector<int>>& relevantGamesGroup = (useGroups) ? (objFlight.gamesGroup[dayIndex]) : (objFlight.gamesSubGroups[dayIndex]);

    if (useGroups) {
        numberOfGroups = objFlight.numberOfGroups[dayIndex];
    }
    else {
        numberOfGroups = 2 * (objFlight.numberOfGroups[dayIndex]);
    }

CodePudding user response：

I would probably write it like this, because I find it quite clear to see:

auto& relevantGamesGroup = useGroups
    ? objFlight.gamesGroup[dayIndex]
    : objFlight.gamesSubGroups[dayIndex];
auto numberOfGroups = useGroups
    ? objFlight.numberOfGroups[dayIndex]
    : objFlight.numberOfGroups[dayIndex] * 2;

CodePudding user response：

This is clean and can be fixed by Joe the Intern if needed be.

using GroupGames = std::vector<std::vector<int>>;
GroupGames* relevantGamesGroup; 
if (useGroups) { 
    relevantGamesGroup = &objFlight.gamesGroup[dayIndex];
}
else {
    relevantGamesGroup = &objFlight.gamesSubGroups[dayIndex];
}

if (useGroups) {
    numberOfGroups = objFlight->numberOfGroups[dayIndex];
}
else {
    numberOfGroups = 2 * (objFlight->numberOfGroups[dayIndex]);
}

Or with the suggestion of @Ted Lyngmo below, it's even cleaner.

using GroupGames = std::vector<std::vector<int>>;
GroupGames* relevantGamesGroup = &objFlight.gamesSubGroups[dayIndex];
int numberOfGroups = 2 * (objFlight->numberOfGroups[dayIndex]);
if (useGroups) { 
    relevantGamesGroup = &objFlight.gamesGroup[dayIndex];
    numberOfGroups = objFlight->numberOfGroups[dayIndex];
}

CodePudding user response：

If you need to use the variables relevantGamesGroup and numberOfGroups after only having checked the condition once, you could create and call a temporary lambda that you make return the necessary pair:

auto&& [relevantGamesGroup, numberOfGroups] =
    [&]() -> std::pair<std::vector<std::vector<int>>&, int>
{
    if (useGroups) return {objFlight.gamesGroup[dayIndex],
                           objFlight.numberOfGroups[dayIndex]};
    return {objFlight.gamesSubGroups[dayIndex],
            2 * objFlight.numberOfGroups[dayIndex]};
}();

// use relevantGamesGroup and numberOfGroups here

An alternative using the ternary/conditional operator instead of using a lambda:

auto&& [relevantGamesGroup, numberOfGroups] =
    useGroups ? std::pair<std::vector<std::vector<int>>&, int>
                    {objFlight.gamesGroup[dayIndex],
                     objFlight.numberOfGroups[dayIndex]}
              : std::pair<std::vector<std::vector<int>>&, int>
                    {objFlight.gamesSubGroups[dayIndex],
                     2 * objFlight.numberOfGroups[dayIndex]};

// use relevantGamesGroup and numberOfGroups here

CodePudding user response：

Not sure whether it is more "elegant", but if you insist of writing only one if/else, then either use a pointer instead of reference for relevantGamesGroup which can be default-initialized and assigned later, or a lambda can help:

auto& relevantGamesGroup = [&]()->decltype(auto){
    if (useGroups) {
        numberOfGroups = objFlight.numberOfGroups[dayIndex];
        return objFlight.gamesGroup[dayIndex];
    } else {
        numberOfGroups = 2 * (objFlight.numberOfGroups[dayIndex]);
        return objFlight.gamesSubGroups[dayIndex];
    }
}();

And for completeness the clearly worse way of doing it with just one ternary operator:

auto& relevantGamesGroup = useGroups
    ? ((void)(numberOfGroups = objFlight.numberOfGroups[dayIndex]),
           objFlight.gamesGroup[dayIndex])
    : ((void)(numberOfGroups = 2 * (objFlight.numberOfGroups[dayIndex])), 
           objFlight.gamesSubGroups[dayIndex]);

((void) cast optional if you are not using some very weird type for numberOfGroups)

CodePudding user response：

This code looks fine to me. Here's how I would rewrite it, but it's mostly a matter of style.

bool useGroups;

// Use of auto
auto& relevantGamesGroup = useGroups ? objFlight.gamesGroup[dayIndex] : objFlight.gamesSubGroups[dayIndex];

numberOfGroups = objFlight.numberOfGroups[dayIndex];
if (useGroups) {
    numberOfGroups *= 2;
}