Using delete on pointers in a a vector

Question

I am a hobby coder who is learning by trying to make a text-RPG game on the console. I am just trying to think about how to design my code right now, and I'm getting stuck on how the items and inventory classes will work together. I have the inventory class defined like so:

class Inventory {
private:
    vector<Item*> items;

public:
    Inventory() {
        items.resize(10);
        for (int i = 0; i < 10; ++i) {
            items[i] = nullptr;//so I can still display an empty inventory
        }
    };
    ~Inventory() {
        for (int i = 0; i < 10; ++i) {
            delete items[i];
        }
    }

    void AddItem(Item* item) {
        for (int i = 0; i < items.size(); ++i) {
            if (items[i] == nullptr) {//if the slot is empty
                items[i] = item;
                break;
            }
        }
    }

    void Show() const {
        for (int i = 0; i < items.size(); ++i) {
            cout << "Slot " << i + 1 << ": ";
            if (items[i] == nullptr) {
                cout << "<Empty Slot>" << endl;
            } else {
                cout << "<" << items[i]->GetName() << ">" << endl;
            }
        }
    }
};

Eventually I'm planning on having enemies dropping loot when they die, so I'll need to create items using new and then transfer them into the player's inventory. My question is, should I just learn about smart pointers and use them, or is the way I've called delete here okay or completely stupid? Or should I be thinking about this code structure differently? Thanks for answering, if you do! I really appreciate it.

The preferred way is `std::vector`, and not to have `null` entries. If you _really_ need pointers, then `std::vector>` — Mooing Duck, Sep 01 '17 at 22:52
https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Mooing Duck, Sep 01 '17 at 22:52
Or, if inventory size is fixed at 10, as it appears in your code, then `std::array, 10>` — Mooing Duck, Sep 01 '17 at 22:52
But I have inherited classes from Item like `class Weapon`, so I'll need the pointers right? Thanks for the answer. — Zeke Willams, Sep 01 '17 at 22:54
@ZekeWillams virtual derivations are indeed a reason for using pointers, but even then, not raw pointers. Use smart pointers and [proper **RAII** concepts.](http://en.cppreference.com/w/cpp/language/raii) — WhozCraig, Sep 01 '17 at 22:55
If the inventory never changes size, look into `std::array`. It's a smarter fixed size array. — user4581301, Sep 01 '17 at 22:56
Yes, I should have used an array. I wasn't thinking. @MooingDuck what is the `optional` thing you used there? Also, what if the inventory size were to change, as if the player upgraded their inventory size or something? — Zeke Willams, Sep 01 '17 at 23:02
If the inventory size is potentially dynamic, then a dynamic container like `std::vector` is in order. Regarding what `std::optional` is/does., you can [read about `std::optional` **here**](http://en.cppreference.com/w/cpp/utility/optional). It is part of C++17, so your toolchain may not be in compliance to make it an... option (poor pun unintended). — WhozCraig, Sep 01 '17 at 23:03
Okay, great. Thank you for the input and knowledge! One last thing, @WhozCraig, is `std::optional` required if I choose to go the `std::array` route? — Zeke Willams, Sep 01 '17 at 23:09

score 2 · Accepted Answer · answered Sep 01 '17 at 23:08

2

The correct method would be to use iterators. Don't pass pointer of 'Item' in

void AddItem(Item* item) method.

You should create an iterator of 'Item' class and pass this iterator to "AddItem" method. Similarly create another method "RemoveItem" that returns an iterator of 'Item' that can be sent to Player's "CollectMoney" method.

If you don't want to use iterators, you can use "Item * item" as well but you have to call a method "RemoveItem" to get the reference of the "Item" so you can pass it to Player. Since the Items are not created inside the class, they should not be deleted in the class so remove the destructor.

PS use this constructor -> Inventory() {items.resize(10,nullptr);}

answered Sep 01 '17 at 23:08

Amsal N

201
2
10

Of all the posts I could find, with similar problems to mine, I have never seen someone mention this. How exactly would you use this? How would I declare an iterator like this and in which class? Thank you for your answer. – Zeke Willams Sep 01 '17 at 23:19
Then follow the above instructions with pointer to "Item", remove the destructor and add "RemoveItem" method. Keep it consistent. You can learn about iterators. They provide encapsulation. Here is a sample iterator : https://gist.github.com/jeetsukumaran/307264 – Amsal N Sep 01 '17 at 23:54

Using delete on pointers in a a vector

1 Answers1