Access private array of pointers to objects from a method in c++

Question

I'm not sure really what to search for to try and find my answer, so please redirect me to any relevant articles and I apologize in advance.

I'm really new to C++ and am trying to make a 3x3 CLI Tile game. I can assign the Tile objects to the private array of pointers in the TileGame constructor and see they have valid memory addresses (or so I think.) My problem is when I run a method to try and access that data. It looks like the the first element in the array has a value, but the others look very odd. I'm sure it's something small, but I'm at a loss as to what I'm doing wrong. I've also tried creating an accessor method, but that only seemed to complicate things.

#include <iostream>
using namespace std;

class Tile
{
    private:
        int position;
        int tileID;
    public:
        Tile(int, int);
};

// tile constructor
Tile::Tile(int p, int id) : position(p), tileID(id) {}

class TileGame
{
    private:
        const int MAX_TILES;
        Tile* tiles[];
    public:
        TileGame();
        void shuffle();
};

// game constructor
TileGame::TileGame() : MAX_TILES(8)
{
    Tile* tiles[MAX_TILES];
    for (int i = 0; i < MAX_TILES; i++)
    {
        tiles[i] = new tile(i, i);
    }

    // spit out the addresses for each
    cout << endl << "From TileGame(): " << endl;
    for (int i = 0; i < MAX_TILES; i++)
    {
        cout << tiles[i] << endl;
    }
}

void TileGame::shuffle()
{
    // spit out the addresses for each
    cout << endl << "From TileGame(): " << endl;
    for (int i = 0; i < MAX_TILES; i++)
    {
        cout << tiles[i] << endl; // <-- This seems to be the problem
    }
}

int main()
{
    TileGame* game = new TileGame();
    game->shuffle();
}

This should compile (I had to retype it, so I hope I didn't make mistakes.) The output that I get is something like:

From TileGame():
0x7f9349c03aa0
0x7f9349c03ab0
0x7f9349c03ac0
0x7f9349c03ad0
0x7f9349c03ae0
0x7f9349c03af0
0x7f9349c03b00
0x7f9349c03b10

From shuffle():
0x7fff9502752c
0x100
0
0
0
0
0
0

I arrived at this point when I kept getting seg faults by trying to access tiles[] values. Thoughts?

if you are allocating 8 size to the 2d array, why not declare it in the class. — Coding Mash, Sep 06 '12 at 17:17

score 4 · Answer 1 · edited May 23 '17 at 12:27

Your constructor is not doing anything to initialise the member called tiles; instead it's populating a local array of the same name. This is discarded, leaking the allocated memory, when the constructor exits. Then you get undefined behaviour (in your case, a segmentation fault) when you try to use the uninitialised member.

The member should be a pointer, rather than an array, if you don't know the size at compile time:

Tile ** tiles;

allocated like:

tiles = new Tile* [MAX_TILES];

Remember to delete each tile (delete tiles[i]), and the array itself (delete [] tiles) in the destructor; and as always when managing resources yourself, remember the Rule of Three.

Better still, use std::vector so you don't have to manage the memory yourself. In general, you should avoid dynamic allocation when you don't need it; for example, the TileGame you create in main could be an automatic variable (TileGame game;) to fix the memory leak you have there.

Thanks for the suggestions. I'm not quite sure I understand exactly what or how to apply these, though. Removing the constructor `tiles[]` does produce the results that I was expecting. Clearly, I have a ways to go. — jktravis, Sep 06 '12 at 18:37

score 1 · Accepted Answer · answered Sep 06 '12 at 17:23

1

The tiles array that is created in the constructor of TileGame is not the tiles array in the Game object. Get rid of the one in the constructor.

Also, too may pointers. The tiles array can hold Tile objects; no need for the indirection and the news. Similarly, there's no need for game to be a pointer. Just create it as a TileGame object.

answered Sep 06 '12 at 17:23

Pete Becker

74,985
8
76
165

To put `Tile` objects rather than pointers in the array, you'd have to make `Tile` default-constructible, or change the array to a vector and emplace them into it. I'm not sure which would be the lesser evil. – Mike Seymour Sep 06 '12 at 17:35
@MikeSeymour - yes, it would require a default constructor. That's far better than the Java-style usage here. – Pete Becker Sep 06 '12 at 17:37
Ah, so it was (initially) a scope issue with the constructor array. I suppose I was thinking that I needed to some how set the maximum number of elements. – jktravis Sep 06 '12 at 18:35
@jktravis - well, you do need to set the size of the array. Unless it's going to vary at runtime, it should probably be `Tile *tiles[MAX_TILES];` or, my preference, `Tile tiles[MAX_TILES];`. – Pete Becker Sep 06 '12 at 18:40
@PeteBecker Where would I set this? I was thinking the constructor, but that was obviously incorrect. – jktravis Sep 06 '12 at 18:48
At the point where it's declared in the class definition. Change `Tile *tiles[]` to one of the two alternatives that I mentioned above. – Pete Becker Sep 06 '12 at 18:54

Access private array of pointers to objects from a method in c++

2 Answers2