views:

1810

answers:

7

In my Dev C++, I am trying to create a 2D Array class that acts like a Grid. But one of the problem is I am unsure what do for the constructor.

When I try to compile, I get the following errors: In constructor 'Grid::Grid(int,int)': 'sqaures' is not a type 'yPos' cannot appear in a constant-expression [Build Error] [grid.o] Error 1

Here is the Header File:

#ifndef GRID_H
#define GRID_H

using namespace std;

class Grid
{
      public:

      Grid(int xPos, int yPos);
      // Constructor
      // POST: Creates the squares of grid; (x,y) coordinates

      private:

      int squares;
      //2D Array
      //the squares; (x,y) coordinates of the grids      
};

#endif

And heres the .cpp file for the functions of grid.h

#include <iostream>
#include "grid.h"

using namespace std;

Grid::Grid(int xPos, int yPos)
{
    squares = new squares[xPos][yPos];
    //Trying to make squares into a 2D array, and turn the values into the arguments
    //into the the x,y coordinates 
}

My constructor in the .cpp files doesn't work and I'm unsure what to do. Does anyone have any solutions?

A: 

Are you sure you got the code right? Your squares is defined as an int, not as an int**? I'm not sure how you're getting this to compile....

EDIT: The error message you are getting results from you defining squares as an int. Therefore, it can only take a single integer number. Your constructor is trying to assign a whole array into it. Are you familiar enough with pointers, arrays, dereferencing and all that stuff? A two dimensional array is generally tricky.

If you write your 2D array well, you can actually use a single array, and just map two-dimensional addresses into a single index.

Uri
He isn't according to his question.
codelogic
I misunderstood his compiler message. I think that he forgot a period somewhere in it.
Uri
+1  A: 

Not directly related to your question, but you should not have using declarations in your header (.h/.hpp) files.

e.g.:

using namespace std;

These belong in cpp files.

See Herb Sutters GOTW (Guru of the Week) #53 for reasons.

grepsedawk
+2  A: 

To avoid lots of memory issues, use a vector of vectors.

In the header

class Grid
{
    ...
    std::vector< std::vector<squares> > squares;
    ...
}

In the .cpp

Grid::Grid(int xPos, int yPos)
{
    squares.resize(xPos);
    for (int i = 0; i < xPos; i++) {
        squares[i].resize(yPos);
    }
}

Later on:

squares[2][3] = square(...);

Or use a vector of vector of smart pointers if you want to new the squares.

David Norman
+1  A: 

There are a few problems with your code. First of all, your member variable "squares" should be a pointer to an int, not an int:

int *squares;

Then, the following line will give an error:

squares = new squares[xPos][yPos];

What you really need is a block of memory for the 2D array:

squares = new squares[xPos * yPos];

Also, you should save the dimensions of this array in member variables (e.g., "sizeX" and "sizeY" )

Now, you have a block of memory which will hold a 2D array of squares. I usually overload the () operator for accessing an element in this array:

int &Grid::operator() (int x, int y)
{
      // you can check array boundaries here
      return squares[y + sizeX*x];
}

If you have problems with the operator stuff, just create a member function instead:

int Grid::get(int x, int y)
{
     // check array bounds
     return squares[y + sizeX*x];
}
void Grid::set(int x, int y, int value)
{
     // check array bounds
     squares[y + sizeX*x] = value;
}

Finally, you need a destructor to free the memory:

Grid::~Grid()
{
     delete [] squares;
}

This is how I like to do it (the "C-with-classes" style). In another answer, David Norman gives a good "Standard C++" way of implementing your class.

Colin
+1  A: 
Grid::Grid(int xPos, int yPos) {
    squares = new squares[xPos][yPos];
    //Trying to make squares into a 2D array, and turn the values into the arguments
    //into the the x,y coordinates 
}

That's of course wrong. you have to do new int[xPos][yPos] . The operator requires you to give it a type. But still then, you are not finished. yPos must be known at compile time. In your example it isn't. The reason is because it becomes part of the type that is returned by the new expression:

int (*squares)[yPos] = new int[xPos][yPos];

As types are static, yPos can't be determined at runtime. What you really want is a vector of int. But i figure you want to do the memory management yourself, because you want to learn the language rules. So go with this:

  1. Make squares a int*: int *squares;
  2. Change the line in the constructor into squares = new int[xPos * yPos];
  3. Add a line like delete[] squares; into your destructor.
  4. Add a copy constructor and copy assigment operator that copies along your memory when your instance is copied.
  5. add a member-function like the below:

Code:

int & get(int x, int y) { return squares[y * yPos + x]; }

Which will give you the integer at the given position. Of course, you can also overload operator[] to have natural access using 2d indices:

class Grid {
    struct proxy {
        proxy(int *row):row(row) { }
        int & operator[](int x) {
            return row[x];
        }
        int * row;
    };

    int * squares;
public:
    proxy operator[](int y) {
        return proxy(squares + yPos * y); 
    }
};

The outer index will select the row, the inner will select the column. If you got to manage the memory right, you can change to better solutions. For your task, boost::multi_array is ideal: Boost.MultiArray

Other problems

Never do using namespace std; in a header file. The reason is that all code that indirectly or directly include that file will automatically also have that line included and thus see all of std::. Name conflicts will happen as soon as code tries to reference names that also happen to be defined by the C++ Standard Library.

Johannes Schaub - litb
+1  A: 

This doesn't work:

squares = new squares[xPos][yPos];

You need:

squares = new (int*)[xPos];
for (int x = 0; x < xPos; ++x) {
     squares[x] = new int[yPos];
}

And personally, that's the wrong way to do it. I prefer

class Grid {

     class Row {
         int* row; // this is a pointer into Grid::grid
         int size; // this equals Grid::col_count and is only here for bounds checking
     public:
         Row(int s, int* r) : size(s), row(r) {}

         int& operator[](int col) {
             if (col >=0 && col < size) return row[col];
             throw OutOfBoundsException();
         }
     };

     int row_count, col_count;
     int* grid;
     Row* rows;

  public:
     Grid(int x, int y) : row_count(x), col_count(y) {
         rows = new (Row*)[row_count];
         grid = new int[row_count*col_count];
         int* grid_walk = grid;
         for (int i = 0; i < row_count; ++i) {
             rows[i] = new Row(col_count, grid_walk);
             grid_walk += col_count;
         }
     }
     ~Grid() { delete[] rows; delete[] grid; }

     Row& operator[](int row) {
         if (row ?= 0 && row < row_count) return rows[row];
         throw OutOfBoundsException();
     }

     int rows() const { return row_count; }
     int cols() const { return col_count; }

};

Grid checkers(8,8);

for (r = 0; r < checkers.row_count(); ++r) {
    for (c = 0; c < checkers.col_count(); ++c) {
        if ((r + c) % 2 == 1) checkers[r][c] = -1; // red space
        else if (r < 3) checkers[r][c] = 1; // player 1
        else if (r >= 5) checkers[r][c] = 2; // player 2
        else checkers[r][c] = 0; // open square
    }
}
// etc.

Hopefully there aren't too many typos.

jmucchiello
+1  A: 

As per David Normans answer, use std::vector. However there is a bug in his answer, the vector should be declared as follows:

class Grid { ...

std::vector< std::vector<int> > squares;

};

You can also initialise it using the vector constructor that takes a size and value:

Grid::Grid(int xPos, int yPos) : squares( xPos, std::vector( yPos, 0 ) ) { }

John W