Classes in C++ Constructor and D'tor

Question

I have two Classes, Player and Game.

class Game
{
        private:
            int maxPlayer;
            Player** playersArray;
        public:
            Game(int maxPlayer);
            ~Game();
}

Each index in playersArray consists of pointers to class Player.I'm not sure though how to make the constructor and destructor of the Class Game. This is my first try but the code isn't working any idea?

Game::Game(int maxPlayer)
{   this->playersArray = new Player*[maxPlayer];
    for(int i=0;i<maxPlayer;i++)
    {
        playersArray[i]=NULL;
    } 
}

Game::~Game() {
    for(int i=0;i<maxPlayer;i++)
    {
        delete[] *playersArray[i];
    }
    delete (playersArray);
}

Avoid explicitly allocating memory like this wherever possible - use a std::vector. — , Jun 07 '18 at 22:08
`delete[] *playersArray[i];` is wrong. It needs to be `delete[] playersArray[i];` -- without the dereference operator. Also, `delete (playersArray);` needs to be `delete [] playersArray;` — R Sahu, Jun 07 '18 at 22:11
Can you describe in more detail what exactly is not working? — Victor Havin, Jun 07 '18 at 22:14
@RSahu: I think it should be delete playersAarray[i]. delete[] is causing vector deleting destructor, which is not necessary for scalar pointers like Player*. — Victor Havin, Jun 07 '18 at 22:20
Also, since you are managing memory, be conscious of the [Rule of Three / Five](https://stackoverflow.com/q/4172722/1553090). In addition to using `std::vector`, you may also want to consider using `std::shared_ptr` or `std::unique_ptr`. — paddy, Jun 07 '18 at 22:30

score 0 · Answer 1 · answered Jun 07 '18 at 22:30

It has been along time since I have programmed in C++. The only error I see for sure is that the constructor has a parameter maxPlayer and never touches the member variable maxPlayer. The destructor uses the member variable maxPlayer, which probably has never been initialized. The constructor ought to assign the parameter to the member variable.

When you delete an array you are supposed to use delete[] but you use delete for a single object. So, I think you have them swapped.

score -1 · Answer 2 · answered Jun 07 '18 at 22:30

-1

OK, since I did not get response to my comment, let me try this: If you modify your code as shown below, does it do what you expect it to do?

class Player
{

};

class Game
{
private:
    int maxPlayer;
    Player** playersArray;
public:
    Game(int maxPlayer);
    ~Game();
};

Game::Game(int maxPlayer)
{
    this->maxPlayer = maxPlayer;
    this->playersArray = new Player*[maxPlayer];
    for (int i = 0;i<maxPlayer;i++)
    {
        playersArray[i] = NULL;
    }
}

Game::~Game() {
    for (int i = 0;i<maxPlayer;i++)
    {
        delete playersArray[i];
    }
    delete[] playersArray;
}

int main()
{
    Game g(10);
    return 0;
}

answered Jun 07 '18 at 22:30

Victor Havin

1,023
7
11

OK. If somebody downvotes an answer, it is considered a good practice to add a comment with explanation. So what is wrong with this code? Basically this is just slightly modified code from the original question which compiles and runs with no errors. So why -1? – Victor Havin Jun 07 '18 at 22:34
I didn't downvote you, but this is not how we do memory management in C++. – Jun 07 '18 at 22:56
@NeilButterworth: Can you be a little more specific? – Victor Havin Jun 07 '18 at 22:58
RAII based containers such as std::vector, and smart pointers. – Jun 07 '18 at 22:59
@NeilButterworth: The question was not about std:vector or smart pointers. The question was: "Why this code does not work?". The original code had some errors that I fixed. This is a working C++ code. We can discuss "good practices" in a separate question. This doesn't make my answer wrong. – Victor Havin Jun 07 '18 at 23:03
It does make it "bad practice", which is presumably why it got a downvote. – Jun 07 '18 at 23:07
2

This maybe creates grounds for downvoting the question, but nor the answer. If somebody asks about pointers, you do not tell him: "You can't use them", because this is not true. Good practice can't be discussed out of context. Not all environments have std: libraries and smart pointers. Many embedded platforms still relay on 'raw' C++. The original question was not "What is the good practice here"? The original question was "Why this doesn't work"? – Victor Havin Jun 07 '18 at 23:13
And how do you know this question is not related to a student term work on arrays and pointers in C++. If this is the case, then the answer "Use vectors" would be totally out of whack. As I said, you can't discuss good practices outside of context. – Victor Havin Jun 07 '18 at 23:16
But nevertheless, you support it. – Victor Havin Jun 07 '18 at 23:17
@NeilButterworth: If you did not downvote it and you do not see flaws other than 'good practices' in it and if you agree with my arguments above, then please be a good nettizen and upvote it. – Victor Havin Jun 07 '18 at 23:20
OK, I have now downvoted it, and I would observe that your "solution" requires a copy constructor and assignment operator to be dealt with. I won't be making any more comments here. – Jun 07 '18 at 23:24
1

I did not add anything that was not defined in the question. Copy constructor and assignment operator take us back to 'good practices'. So you decided to 'punish me' for arguing with you. It is your choise, but this is a bad karma. – Victor Havin Jun 07 '18 at 23:29
You might have noticed that getting upset about receiving downvotes does not attract any sympathy. It suggests you care more about your reputation than helping others or learning. You received 3 downvotes. Mine is one of them, for these reasons: (1) You asked for clarification, didn't get it and then answered anyway; (2) Your answer was basically the question "does this work?" which is not particularly authoritative; (3) There was no explanation of what the problem is, nor what the fix is and why it works. Neither was was there any indication that you understand the safety of this code. – paddy Jun 08 '18 at 01:30
@paddy: I respectfully disagree. I do not really care about my reputation here, since I am way beyond starting my career and closer to the other end. I only answer questions if I believe I can help. I did ask for clarification and did not get it. Is it my fault? Without the clarification I answered to the best of my abilities based on the information provided. The code was pretty much self-explanatory. I wanted to confirm that my solution helped, so I asked "does it do what you expect it to do? If you consider this grounds for downvote - fine. It's your karma after all. I rest my case. – Victor Havin Jun 08 '18 at 01:43
A downvote is an opportunity to learn. As long as you continue insisting that it is some kind of punishment or personal attack on you, no learning is gonna happen. – paddy Jun 08 '18 at 01:56
Well, enlighten me: What am I supposed to learn from your downvote exactly? And what should I learn from other two downvoters, who did not even come up with a plausible explanation? – Victor Havin Jun 08 '18 at 02:00

Classes in C++ Constructor and D'tor

2 Answers2

Linked