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
– Mert Mertce

2015-08-12 07:26:01 +00:00
Commented Aug 12, 2015 at 7:26 — Mert Mertce
– Mert Mertce, Commented Aug 12, 2015 at 7:26

Mert Mertce · Accepted Answer · 2015-08-12 07:38:35Z

1

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

answered Aug 12, 2015 at 7:38

Mert Mertce

1,6343 gold badges21 silver badges32 bronze badges

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

2 Comments

Sawyer Adlai Vierra-Hatch Over a year ago

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.

Mert Mertce Over a year ago

@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.

not-a-robot · Accepted Answer · 2015-08-12 07:32:59Z

0

You never assign anything to itemName.

Change switch(itemName) to switch(item).

answered Aug 12, 2015 at 7:32

not-a-robot

3213 silver badges14 bronze badges

Comments

Community · Accepted Answer · 2017-05-23 11:51:20Z

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, 2017 at 11:51

CommunityBot

11 silver badge

answered Aug 12, 2015 at 7:41

legends2k

33.2k28 gold badges131 silver badges238 bronze badges

Comments

user3483948 · Accepted Answer · 2015-08-12 07:42:21Z

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, 2015 at 7:42

user3483948

366 bronze badges

2 Comments

Sawyer Adlai Vierra-Hatch Over a year ago

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.

user3483948 Over a year ago

Bad things happen mate. That's the best way to learn though. Good luck with your work

Collectives™ on Stack Overflow

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

4 Answers 4

2 Comments

Comments

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related