Jump to content

  • Log In with Google      Sign In   
  • Create Account

const return problem


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
7 replies to this topic

#1 rakketh   Members   -  Reputation: 192

Like
0Likes
Like

Posted 15 January 2009 - 10:15 AM

I'm having some trouble working out how to return consts correctly to stop the returned value from being modifyable, specifically when using pointers. I have a GameState class that contains a dynamically allocated 2d grid. I want to be able to return a non-modifyable version of the grid to classes such as the graphics renderer. Here is what I have:
#pragma once

struct GameSize {
	int x, y;
};

class GameState
{
	GameState(int x, int y);
	~GameState();

	const GameSize * getGameSize() { return &gameSize; }
	const bool ** getGrid() { return grid; }
	void changeSquare(int x, int y);

private:
	GameSize gameSize;
	bool **grid;
};


The first return works fine, returning a pointer to the GameState's GameSize object which cannot be modified (please correct me if that is wrong). The second return gives the following error: 3>c:\documents and settings\administrator\my documents\visual studio 2008\projects\testgame\testgame\gamestate.h(13) : error C2440: 'return' : cannot convert from 'bool **' to 'const bool **' I've returned grid instead of &grid since it's already a pointer so passing it by value isn't an overhead.

Sponsor:

#2 rip-off   Moderators   -  Reputation: 8222

Like
0Likes
Like

Posted 15 January 2009 - 10:23 AM

An evil way of avoiding the question, but if you used an actual type for the grid (say, boost::multi_array) and returned a const reference to this instance it would work as you expect. As it is your class doesn't comply with the Rule of Three. An RAII wrapper around the array would help.

Also you are returning a pointer to a GameState when you could return a reference.

#3 Brother Bob   Moderators   -  Reputation: 8196

Like
0Likes
Like

Posted 15 January 2009 - 10:55 AM

Follow rip-off's advice, but just to answer the question; you need to add const to all levels of the double pointer.

const bool * const * getGrid() { return grid; }

Otherwise the grid-array can be modified via the first indirection, which is not const, and so you can reassign the first level of pointers and therefore change the array you just made constant.

#4 rakketh   Members   -  Reputation: 192

Like
0Likes
Like

Posted 15 January 2009 - 01:24 PM

Thanks for the replies and the links. That const 2-d array gets complicated fast! I'm quite new to cpp, being a Java developer by trade but wanting to learn cpp for games development as a hobby. I've read up on references now and see that they are a much better option than pointers in the way I am using them here.

I've refactored my code into a new class based on your advice:

GameGrid.h

#pragma once

struct GameSize {
GameSize(int x, int y) : x(x), y(y) {}
~GameSize() {}

int x, y;
};

struct GameSquare {
GameSquare() { state = false; }
bool state;
};

class GameGrid
{
public:
GameGrid(const GameGrid &source);
GameGrid(int x, int y);
~GameGrid(void);
GameGrid & operator = (const GameGrid & other);

const GameSize& getGameSize() { return size; }
const GameSquare& getGameSquare(int x, int y);
void changeGameSquare(int x, int y);

private:
GameSize size;
GameSquare **squares;
};



GameGrid.cpp

#include "StdAfx.h"
#include "GameGrid.h"

GameGrid::GameGrid(const GameGrid &source) : size(source.size) {
squares = new GameSquare* [size.x];
squares[0] = new GameSquare[size.x * size.y];

for(int ix = 0; ix < size.x; ix++)
{
for(int iy = 0; iy < size.y; iy++)
{
squares[ix][iy].state = source.squares[ix][iy].state;
}
}
}

GameGrid::GameGrid(int x, int y) : size(x, y)
{
squares = new GameSquare* [x];
squares[0] = new GameSquare[x * y];

for(int ix = 0; ix < x; ix++)
{
for(int iy = 0; iy < y; iy++)
{
squares[ix][iy].state = (rand() % 2 == 1);
}
}
}

GameGrid::~GameGrid(void)
{
if(squares != NULL)
{
delete[] squares[0];
delete[] squares;
}
}

GameGrid &GameGrid::operator =(const GameGrid & other)
{
if(this != &other) {
GameSquare **tempSquares;
tempSquares = new GameSquare* [size.x];
tempSquares[0] = new GameSquare[size.x * size.y];

for(int ix = 0; ix < size.x; ix++)
{
for(int iy = 0; iy < size.y; iy++)
{
tempSquares[ix][iy].state = other.squares[ix][iy].state;
}
}

if(squares != NULL)
{
delete[] squares[0];
delete[] squares;
}
squares = tempSquares;
size = other.size;
}
return *this;
}

const GameSquare& GameGrid::getGameSquare(int x, int y) {
// if(x < 0 || x > size.x - 1 || y < 0 || y > size.y - 1) return NULL;

return squares[x][y];
}

void GameGrid::changeGameSquare(int x, int y) {
if(x > 0 && x < size.x - 1 && y > 0 && y < size.y - 1)
{
squares[x][y].state = !squares[x][y].state;
}
}



Does this look like a correct implementation of the rule of 3?

There is a commented out line which I'm not sure how to handle. In Java, this works since everything is treated as a reference. In this instance should I be using a pointer instead of a reference, or is there a better method? (I am of the understanding that cpp exceptions are slow!)

#5 Cornstalks   Crossbones+   -  Reputation: 6990

Like
0Likes
Like

Posted 15 January 2009 - 02:00 PM

You can't return a NULL reference (which is probably why you've commented it out). Instead, you should be using the assert method. Learn to love the assert method.

As for your Rule of 3 implementation, it looks kind of ok. I say kind of because I don't think you're doing what you want to do. You aren't making much of a grid. Look at this line:

squares = new GameSquare* [x];
squares[0] = new GameSquare[x * y];

The second line looks all wrong. In it, you're only allocating memory for the first row, and even then you're allocating memory for x * y GameSqaures. Really, you should loop over squares and create a column of height y for every row. And don't forget to delete [] each one that you new! (Sorry, I know this is a general statement, but I want to you try to think about it).

For some reason I'm getting the uneasy feeling of the whole x-y vs row-column battle for 2D arrays. Will someone make sure that his/her x's and y's are in the right order? I don't have enough time.
[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

#6 Guthur   Members   -  Reputation: 100

Like
0Likes
Like

Posted 15 January 2009 - 02:56 PM


void GameGrid::changeGameSquare(int x, int y) {
if(x > 0 && x < size.x - 1 && y > 0 && y < size.y - 1)
{
squares[x][y].state = !squares[x][y].state;
}
}




I think you want >= here not just >, unless the first element of each column and row is to be ignored.



struct GameSize {
GameSize(int x, int y) : x(x), y(y) {}
....
int x, y;
};



Is this ok? looks like self assignment, not sure how this is interpreted by the compiler to be honest someone else needs to step in.

#7 Aardvajk   Crossbones+   -  Reputation: 5982

Like
0Likes
Like

Posted 15 January 2009 - 04:03 PM

If your grid is regular (i.e. all rows the same length), it is far easier to allocate a single block of contiguous memory and use a (y*width)+x formula to index, avoiding all the issues with double pointers. You could also wrap the grid in a class to make it easier to deal with returning it:


class grid
{
private:
bool *data; int w,h;

public:
grid(int w,int h) : w(w),h(h) { data=new bool[w*h]; }
~grid(){ delete [] data; }

bool &operator()(int x,int y){ return data[(y*w)+x]; }
bool operator()(int x,int y) const { return data[(y*w)+x]; }

int width() const { return w; }
int height() const { return h; }
};

struct gamesize { int x,y; };

class gamestate
{
private:
grid g;

public:
gamestate(int w,int h) : g(w,h) { }

// might as well use a temporary for gamesize
gamesize getgamesize() const { return gamesize(g.width(),g.height()); }

const grid &getgrid() const { return g; }
};

void f(gamestate &g)
{
int w=g.getgamesize().x;

g.grid(4,2)=false;
}




I'd normally now suggest that you replace the dynamic array of bools with a std::vector<bool>, but IIRC, a compiler is free to specialise std::vector<bool> with some kind of bitset, and this might cause complications if you wanted to try to address an individual element.

#8 rakketh   Members   -  Reputation: 192

Like
0Likes
Like

Posted 16 January 2009 - 02:33 AM

Quote:
Will someone make sure that his/her x's and y's are in the right order?

his :D Don't worry too much about x/y battling as that is something that will be worked out when I debug if it is wrong. I'm mainly after advice on good practices.

Quote:
squares = new GameSquare* [x];
squares[0] = new GameSquare[x * y];

I found this on a website when I was looking up dynamically allocated 2d arrays. The idea behind it is that you save overhead by only calling new and delete twice. I agree that it doesn't look right, but I went along with it. After another search I see that I've missed a vital point, the full example that I now find is:

template < typename T >
T **Allocate2DArray( int nRows, int nCols)
{
//(step 1) allocate memory for array of elements of column
T **ppi = new T*[nRows];

//(step 2) allocate memory for array of elements of each row
T *curPtr = new T [nRows * nCols];

// Now point the pointers in the right place
for( int i = 0; i < nRows; ++i)
{
*(ppi + i) = curPtr;
curPtr += nCols;
}
return ppi;
}

template < typename T >
void Free2DArray(T** Array)
{
delete [] *Array;
delete [] Array;
}


I agree, however, that this is unwieldy and will change the grid to a vector using the maths example provided by EasilyConfused. I have changed the bool into a GameSquare struct with a state member variable to make it easier to change its implementation in the future and to avoid the vector<bool> potential pitfalls.


Quote:
I think you want >= here not just >, unless the first element of each column and row is to be ignored.

Yes, this is wrong, should be >= 0, < size. I blame tiredness :) thanks for pointing it out.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS