Declaring a variable in an if-else block in C++

Question

I'm trying to declare a variable in an if-else block as follows:

int main(int argc, char *argv[]) {

    if (argv[3] == string("simple")) {
        Player & player = *get_Simple();
    } else if (argv[3] == string("counting")) {
        Player & player = *get_Counting();
    } else if (argv[3] == string("competitor")) {
        Player & player = *get_Competitor();
    }

    // More code
}

But, I'm getting the following errors when I try to compile:

driver.cpp:38: error: unused variable ‘player’
driver.cpp:40: error: unused variable ‘player’
driver.cpp:42: error: unused variable ‘player’
driver.cpp:45: error: ‘player’ was not declared in this scope

Any ideas?

score 21 · Accepted Answer · answered Nov 24 '09 at 23:59

21

Your problem is that player falls out of scope in each if / else if block.

You need to declare your variable above all of the if statements.

But you can't use a reference for that because you must initialize a reference right away.

Instead you probably want something like this:

int main(int argc, char *argv[]) {

    Player * pPlayer = NULL;
    if (argv[3] == string("simple")) {
        pPlayer = get_Simple();
    } else if (argv[3] == string("counting")) {
        pPlayer = get_Counting();
    } else if (argv[3] == string("competitor")) {
        pPlayer = get_Competitor();
    }

    //Then if you really want to...
    Player &player = *pPlayer;

}

answered Nov 24 '09 at 23:59

Brian R. Bondy

314,085
114
576
619

4

This advice is sound, but I'd suggest that you could avoid the whole pointer/reference switcheroo at the end by having a factory method that takes your type parameter and returns a `Player*`. So your main method becomes `Player &player = *getPlayerByType(argv[3]);`, and the `if` statements in `getPlayerByType()` each return directly, thus avoiding all that local variable ugliness. – bradheintz Nov 25 '09 at 00:25
2

@bradheintz: That advice does not help in the slightest (in-fact it clutters up the answer by providing spurious advice). It is obvious that the OP is actually trying to do exactly what you say (and return a reference (rather than a pointer as in your solution)). But has posted the simplist compilable version of the question here. If you want to post an answer do that, then we can at least give you a negative mark for not answering the question. – Martin York Nov 25 '09 at 01:21
I'm not sure what's spurious about packaging the object creation into a single-responsibility function, eliminating an unnecessary local variable, and making the main() function more scannable - but if it makes you happier to call my advice "spurious", or to call a suggested refinement of Brian R. Bondy's good advice "cluttering" without supporting the assertion, go for it. – bradheintz Nov 25 '09 at 18:00

score 17 · Answer 2 · answered Nov 25 '09 at 00:40

17

Others have suggested pointers. However, the conditional operator may be used as well.

Player & player = argv[3] == string("simple") ? get_Simple()
                : argv[3] == string("counting") ? get_Counting() 
                : get_Competitor();

answered Nov 25 '09 at 00:40

rlbond

59,991
50
166
218

Parappa · Answer 3 · 2009-11-25T23:12:09.000

3

If you put a static variable inside of a scope, delimited by { }, then that variable will no longer be available when the scope ends.

Try this instead:

int main(int argc, char *argv[]) {

    // TODO: validate argc and argv here
    if (argc < 3) {
        printf("error: not enough arguments\n");
        exit(1);
    }

    Player* player_ptr = NULL;
    if (argv[3] == string("simple")) {
        player_ptr = get_Simple();
    } else if (argv[3] == string("counting")) {
        player_ptr = get_Counting();
    } else if (argv[3] == string("competitor")) {
        player_ptr = get_Competitor();
    }

    if (!player_ptr) {
        printf("error: invalid argument %s\n", argv[3]);
        exit(1);
    }

    Player& player = *player_ptr;

    // More code
}

edited Nov 25 '09 at 23:12

answered Nov 24 '09 at 23:59

Parappa

7,218
2
32
37

1

A reference must have an initial value. – Jim Buck Nov 25 '09 at 00:04
Yup, I messed up. Tried to fix it. – Parappa Nov 25 '09 at 00:09
1

You also need to NULL the pointer, otherwise the assert might be completely off, since it might skip all of the clauses. And in the same spirit, since the pointer can remain NULL, referencing might be a problem. – laura Nov 25 '09 at 12:16
1

And, of course, you should not use assertions to validate user inputs – erikkallen Nov 25 '09 at 22:45

score 2 · Answer 4 · answered Nov 25 '09 at 00:06

You've declared three separate player variables in three different scopes, and the error message is saying exactly what it means.

You need to declare a single player variable outside the if-statement and assign the result. This is tricky, since player is a reference--you must initialize it once.

You can put the if-statement in a function (say GetPlayer()) that returns a pointer to the object, and then initialize player with *GetPlayer().

score 0 · Answer 5 · answered Nov 24 '09 at 23:59

0

In

if (argv[3] == string("simple")) {
    Player & player = *get_Simple();
}

The variable only exists between the {}'s. When you get to the }, the variable has not been used and will be discarded, never having been used.

answered Nov 24 '09 at 23:59

S.Lott

359,791
75
487
757

score 0 · Answer 6 · answered Nov 25 '09 at 12:11

#include <map>

int main(int argc, char **argv)
{
    typedef std::map<std::string, Player*(*)()> lookup;
    lookup mapping;

    mapping["simple"] = get_Simple;
    mapping["counting"] = get_Counting;
    mapping["competitor"] = get_Competitor;

    lookup::const_iterator it = mapping.find(argv[3]);
    if (it == mapping.end())
    {
        std::cout << "illegal argument\n";
    }
    else
    {
        Player& player = *it->second();
        // more code
    }
}

Declaring a variable in an if-else block in C++

6 Answers6

Linked