const return problem

Started by
6 comments, last by rakketh 15 years, 3 months ago
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.
Advertisement
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.
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.
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 oncestruct 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!)
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.
[size=2][ 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 ]
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.
Innovation not reiterationIf at any point I look as if I know what I'm doing don't worry it was probably an accident.
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.
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.

This topic is closed to new replies.

Advertisement