Why is my class constructor being called twice?

Question

I've got a base class Container with a derived class Player_Inventory. There can only be one Player_Inventory so my code throws an exception if for some reason a second one is created.

The problem I'm having is that my code is failing my test as it throws the exception even on what is supposed to be the very first construction of the Player_Inventory class. I've debugged the code and two things are happening which I don't quite understand - the number attribute is not tracked by the debugger (at least not in the GUI on VSC), and it seems that right after hitting the first REQUIRE statement, the constructor is called again, thus triggering the exception.

Can anyone help?

After rewriting my constructor method, I'm still getting a similar error.

My revised code is as follows:

containers.h

#include<iostream>
#include<string>
#include<vector>

class Item {    // Placeholder class for items
    public:
        std::string name;
        Item(std::string n) : name{n} {};
};

class Container {
    protected:
        std::string name;
        std::string description;
        std::vector<Item> contents;

    public:
        Container(std::string, std::string);
        std::string get_name() {return name;}
        std::string get_description() {return description;}
        std::vector<Item> get_contents() {return contents;}
};

containers.cpp (there are more methods defined in this file which aren't used here)

#include<iostream>
#include<string>
#include "containers.h"

Container::Container(std::string n, std::string desc) : name{n}, description{desc} {};

player_inventory.h

#include "containers.h"

class Player_Inventory : public Container {

    public:
        static int number;
        Player_Inventory(std::string, std::string);
};

player_inventory.cpp

#include<iostream>
#include<stdexcept>
#include "player_inventory.h"

Player_Inventory::Player_Inventory(std::string n, std::string desc): Container(n, desc) {
    number += 1;
    if (number > 1){
        throw std::invalid_argument("You can only have one inventory!");
    }
};

int Player_Inventory::number = 0;

test_file.cpp

#include "../lib/Catch2/catch.hpp"
#include "player_inventory.h"
#include<iostream>
#include<string>
#include<vector>

SCENARIO("A player can have an inventory.") {

    WHEN("A player inventory  is created.") {
        Player_Inventory myInventory("My Inventory", "Inventory for the player");

        THEN("The created inventory has the correct attribute values.") {
            REQUIRE(myInventory.get_name() == "My Inventory");
            REQUIRE(myInventory.get_description() == "Inventory for the player");
            REQUIRE(myInventory.get_contents().empty());
        }  // The code works fine when only up to here is included

        AND_THEN("Only one player inventory can exist.") { // as soon as this line is included it tries to create another player_inventory object, causing the fail
            REQUIRE_THROWS((Player_Inventory myOtherInventory("Second Inventory", "Testing for another one"))); // These two lines were not included but I've included them here as this is the test I wanted to run
            REQUIRE(myInventory.get_number() == 1);
        }
    }
}

You have an interesting combination of over-minimizing and including code that can't help solve the problem. I bet if you build a better example, read [mre] for inspiration, you'll find the problem without any more help. If not, post the better example. — user4581301, Feb 18 '21 at 16:01
Where are you initializing `Player_Inventory::number`? Also, this is a dreadful way of creating a singleton. Which is a dreadful way to program in general. — Blindy, Feb 18 '21 at 16:01
@Blindy can you point me in the direction of a better way to approach this? — askman, Feb 18 '21 at 16:37
@askman [a better singleton](https://stackoverflow.com/a/1008289/4581301). But you probably don't need it. Unfortunately to know what you should be doing we need to see more of how `Player_Inventory` is used. It's possible someone can offer a compile-time solution that traps multiple instances and emits an error when you build the program. — user4581301, Feb 18 '21 at 17:28
I completely misinterpreted the question. I'm glad my wrong-answer solved another problem you didn't realize you were facing, but since I don't have catch 2 I can't help you nail down exactly where the extra construction or damage to the value of `number` is occurring. Pop a breakpoint in the constructor and make absolutely certain it's not being called twice. [Here's how to see more than just the local variables in Visual Studio Code.](https://stackoverflow.com/questions/49956025) If that doesn't let you see `number`, the sucker might be optimized out. Print it to the console. — user4581301, Feb 18 '21 at 18:31
Consider making an example without Catch2 and see if you get the same deviant behaviour. — user4581301, Feb 18 '21 at 18:33

score 1 · Accepted Answer · answered Feb 18 '21 at 16:02

1

Not sure if related, but that's how you should call the Base constructor:

Player_Inventory(std::string n, std::string desc) : Container(n, desc) {
}

answered Feb 18 '21 at 16:02

m88

1,968
6
14

This actually solved it with no other changes but I'm gonna read up on the other suggestions too. – askman Feb 18 '21 at 16:10

Why is my class constructor being called twice?

1 Answers1