C++ initialize reference and assign variable together

Question

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

There's nothing wrong with the way you have it written. You could have another ternary after the reference assignment, i.e., numberOfGroups = (useGroups ? 2 : 1) * objFlight.numberOfGroups[dayIndex]; but this is arguably less clear, and may be slightly less efficient, depending on how the compiler optimizes. — Charles Savoie
– Charles Savoie, Commented Dec 28, 2021 at 16:36
Is there a typo in 2 * line? Should it use SubGroups there? — rturrado
– rturrado, Commented Dec 28, 2021 at 16:38
@CharlesSavoie objFlight.numberOfGroups[dayIndex] * (useGroups + 1) if you really want to be a smartass :) — Rotem
– Rotem, Commented Dec 28, 2021 at 16:39

rturrado · Accepted Answer · 2021-12-29 13:47:11Z

5

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

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

edited Dec 29, 2021 at 13:47

answered Dec 28, 2021 at 16:51

rturrado

8,2097 gold badges44 silver badges65 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Ted Lyngmo · Accepted Answer · 2021-12-29 12:14:36Z

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

If you use this kind of construct a lot, creating a helper function could simplify it:

#include <tuple>

template<class... Ts>
std::tuple<Ts...> ternary(bool cond, std::tuple<Ts...>&& True,
                                     std::tuple<Ts...>&& False) {
    return cond ? True : False;
}

You'd then supply the wanted types as template parameters and use structured bindings to extract the selected values / references just like above:

int main() {
    int a1 = 1, b1 = 2, c1 = 3;
    int a2 = 40, b2 = 50, c2 = 60;

    auto&&[a,b,c] = ternary<int&,int&,int&>(true, {a1,b1,c1}, {a2,b2,c2});

    std::cout << a << b << c << '\n';
    ++a; ++b; ++c; // these are references to a1, b1 and c1
    std::cout << a1 << b1 << c1 << '\n';
}

Output:

123
234

With the types in your question, it could look like this:

void func(bool useGroups) {
    auto&& [relevantGamesGroup, numberOfGroups] =
      ternary<std::vector<std::vector<int>>&, int>(useGroups,
        {objFlight.gamesGroup[dayIndex],     objFlight.numberOfGroups[dayIndex]},
        {objFlight.gamesSubGroups[dayIndex], 2 * objFlight.numberOfGroups[dayIndex]});

    // use relevantGamesGroup and numberOfGroups here
}

Sure it works that way? I'm having trouble replicate: godbolt.org/z/xGcjEjo3q Can you have a look?
@Jellyboy Yes, you just forgot to call the lambda: godbolt.org/z/qdTf8Yfsd

user17732522 · Accepted Answer · 2021-12-29 11:53:53Z

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];
    }
}();

(Note that ->decltype(auto) is important here, since the lambda will otherwise return by-value, not by-reference.)

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)

G'k · Accepted Answer · 2021-12-28 16:35:41Z

1

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

answered Dec 28, 2021 at 16:35

G'k

661 silver badge7 bronze badges

Comments

score 1 · Accepted Answer · 2021-12-28 17:19:21Z

1

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

edited Dec 28, 2021 at 17:19

answered Dec 28, 2021 at 16:35

user8143588

7 Comments

user8143588 Over a year ago

@TedLyngmo good advice, thank you!

Ted Lyngmo Over a year ago

You're welcome! You could add auto& relevantGamesGroup = *relevantGamesGroupPtr; to get a nice reference after the if too.

user8143588 Over a year ago

@TedLyngmo I have a personal preference not to use "auto" unless strictly necessary. I work with mission critical systems and I have been bitten more than once by folks using auto and creating the wrong variable type with bad consequences. I like to go the extra mile and specify crystal clear what every type should be.

Ted Lyngmo Over a year ago

@Jellyboy Oups :-) Well, you could be explicit and make it GroupGames& relevantGamesGroup = *relevantGamesGroupPtr; instead.

Rotem Over a year ago

@Jellyboy sure, here's one with relevance. In your first block of code I can see no reason to have two separate if clauses. It adds nothing to readability.

|

Collectives™ on Stack Overflow

C++ initialize reference and assign variable together

5 Answers 5

Comments

2 Comments

Comments

Comments

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

2 Comments

Comments

Comments

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related