For loop array iteration, if loop tests if indices is true/false, evaluating true incorrectly

Question

I'm working on an inventory system as a challenge for the amount of c++ I currently know, I'm using an array with enumerated indices and initializing with value true or false. This is the code:

using namespace std;

enum items
{
    LASER_RIFLE,
    LASER_SWORD,
    LASER_PISTOL,
    PLASMA_LAUNCHER,
    MAX_ITEMS
};


void playerInventory()
{
    bool items[MAX_ITEMS];
    items[LASER_RIFLE] = true;
    items[LASER_SWORD] = false;
    items[LASER_PISTOL] = false;
    items[PLASMA_LAUNCHER] = true;

    int itemName;
    for (int item = 0; item <= MAX_ITEMS; ++item)
        if (items[item] == true)
        {
            switch (itemName)
            {
            case 1:
                cout << "Laser Rifle\n";
                break;
            case 2:
                cout << "Laser Sword\n";
                break;
            case 3:
                cout << "Laser Pistol\n";
                break;
            case 4:
                cout << "Plasma Launcher \n";
                break;
            }
        }
        else
            cout << "Not in Inventory\n";
}

The statement only evaluates true for Laser pistol, and false for everything else. I cannot figure out why this is.

you put swtich(itemName). never changes in loop. – Mert Mertce Aug 12 '15 at 07:26 — Mert Mertce, Aug 12 '15 at 07:26

score 1 · Answer 1 · answered Aug 12 '15 at 07:38

You are switching for itemName. You should switch for item
Enumeration on default starts with 0, not 1. Your cases should start from 0.
In for loop you have to check against < not <=.

Here is the result:

#include <iostream>
using namespace std;

enum items
{
    LASER_RIFLE,
    LASER_SWORD,
    LASER_PISTOL,
    PLASMA_LAUNCHER,
    MAX_ITEMS
};


void playerInventory()
{
    bool items[MAX_ITEMS];
    items[LASER_RIFLE] = true;
    items[LASER_SWORD] = false;
    items[LASER_PISTOL] = false;
    items[PLASMA_LAUNCHER] = true;

    int itemName;
    for (int item = 0; item < MAX_ITEMS; ++item) {
        if (items[item] == true)
        {
            switch (item)
            {
            case 0:
                cout << "Laser Rifle\n";
                break;
            case 1:
                cout << "Laser Sword\n";
                break;
            case 2:
                cout << "Laser Pistol\n";
                break;
            case 3:
                cout << "Plasma Launcher \n";
                break;
            }
        }
        else {
            cout << "Not in Inventory\n";
        }
    }
}

int main() {
    playerInventory();
    return 0;
}

See: ideone

Thank you for the fast response, I appreciate the thoroughness of your answer. Like I said, I'm relatively new to C++, I understand the enumeration problem and the itemName problem, but what i don't get is why it was evaluating to true for only Laser Pistol, i know initialized variables screw things up, but it was still not working when i had itemName = item. Thank's for your help. — Sawyer Adlai Vierra-Hatch, Aug 12 '15 at 07:50
@SawyerAdlaiVierra-Hatch "it was evaluating to true for only Laser Pistol" is false. It was not evaluating true for Laser Pistol. The code was entering in if case when item was 0 and 3. There was no problem there. You just saw "Laser Pistol" print coincidentally, not because it was evaluated to true. How could itemName be evaluated as 3 and you saw that print.. Who knows? It tells you, it is undefined behaviour. — Mert Mertce, Aug 12 '15 at 08:28

score 0 · Answer 2 · answered Aug 12 '15 at 07:32

0

You never assign anything to itemName.

Change switch(itemName) to switch(item).

answered Aug 12 '15 at 07:32

not-a-robot

321
1
14

score 0 · Answer 3 · edited May 23 '17 at 11:51

0

You've two places invoking undefined behaviour:

item <= MAX_ITEMS should be item < MAX_ITEMS; otherwise you'd be accessing the array beyond its bounds.
Accessing an uninitialized variable: itemName.

Once you've undefined behaviour, all bets are off. Do not expect any sane output from the program.

edited May 23 '17 at 11:51

Community

1
1

answered Aug 12 '15 at 07:41

legends2k

31,634
25
118
222

score 0 · Answer 4 · answered Aug 12 '15 at 07:42

0

itemName is never initialized so use itemName INSIDE the for scope and equalize the true ones with it or directly change it with item.

Oh and you are looking for a place that is beyond your arrays bounds. The array's length may be MAX_ITEMS long but your indices start from 0. So you should use item < MAX_ITEMS instead of item <= MAX_ITEMS.

Btw: you may want to use default for your switch-case structure. Not necessary for this one but it can be a lifesaver.

answered Aug 12 '15 at 07:42

user3483948

36
6

I can't believe I forgot that, I guess it just come with starting out. I hate it when i forget silly stuff like that though. Thanks for your help. – Sawyer Adlai Vierra-Hatch Aug 12 '15 at 07:50
Bad things happen mate. That's the best way to learn though. Good luck with your work – user3483948 Aug 12 '15 at 07:53

For loop array iteration, if loop tests if indices is true/false, evaluating true incorrectly

4 Answers4