Hello folks

I'm trying to simulate a magic square algorithm, but having some problems. The program successfully creates the magic square, but when I'm trying to print it, the VC2005 compiler says "vector subscript out of range", which it shouldnt be.
Any help would be appreciated.
Cheers

Here's the code:

class Magicsquare{

public:
    Magicsquare(int n);
    void display(void);

private:
    vector<vector<int> > square; //storing #s here

int totalsize;  //stuff for generating the square
    int nsqr;
    int i,j;
    };

MagicSquare::MagicSquare(int n)
{
   totalsize=n;

vector<int> v(n);
   vector<vector<int> >   square(n,v);    

  nsqr = n * n;             //this part runs ok
  i=0;
  j=n/2;    

  for (int k=1; k<=nsqr; ++k) 
  {
    data[i][j] = k;

    i--;
    j++;

    if (k%n == 0) 
    { 
      i += 2; 
      --j; 
    }
    else 
    {
      if (j==n) 
        j -= n;
      else if (i<0) 
        i += n;
    }
  }
}

void Magicsquare::display(void)        //this one doesn't
{
   for (int i=0; i<sz; i++) 
  {
    for (int j=0; j<sz; j++)
         cout << data[i][j] << endl;
        cout<<endl;
  }
  }

Where is the variable sz defined and given avalue, and what value does it have in relation to dimensions of the the variable called data?

Also, please indent code so it is easier to read.

Here's my try for the properly indented code with the display function properly pasted. Sorry for the inconveniences, but pasting it from VC doesn't preserve whitespace, whis is very annoying!

#include"stdafx.h"
#include<iostream>
#include<vector>
usingnamespace std;
 
 
 
 
class MagicSquare{
 
private:
vector<vector<int> > data;
 
int sz; // !! sz is the size of the magic square, therefore the demensions of 
// the data 'matrix' 
int nsqr;
int i,j;
 
public:
MagicSquare(int n);
void displaySquare(void);
};
 
int main()
{
MagicSquare test(7);
test.displaySquare();
 
return 0;
}
 
 
MagicSquare::MagicSquare(int n)
{
sz=n;
vector<int> v(n); 
vector<vector<int> > data(n,v); 
 
 
nsqr = n * n;
i=0;
j=n/2; 
for (int k=1; k<=nsqr; ++k) 
{
data[i][j] = k;
i--;
j++;
if (k%n == 0) 
{ 
i += 2; 
--j; 
}
else 
{
if (j==n) 
j -= n;
else if (i<0) 
i += n;
}
}
}
 
 
 
 
void MagicSquare::displaySquare(void)
{
cout<<"\n\n\n\n####in printing function\n\n";
for (int i=0; i<sz; i++) 
{
for (int j=0; j<sz; j++)
cout << data[i][j] << endl;
cout<<endl;
}
 
}

Two problems I can see:

• Your memory allocation is not proper. Each of your rows share a common vector variable because of the stmt:

vector<int> v(n);
vector<vector<int> > data(n,v);

Here the memory for each inner row is getting allocated only once and your two dimensional matrix gets populated with the same vector.

You need to change it to:

vector<vector<int> > data(n,vector<int> (n));

So that the memory allocation occurs for each row rather than only once for all rows.

• Your logic is skewed. The line :

i=0;
    j=n/2;
    for (int k=1; k<=nsqr; ++k)
    {
        data[i][j] = k;
        i--;

is bound to produce an out of bounds error since you are decemeneting i which already is zero, resulting in its value becoming -1, which is not a valid index.

Better use size_t instead of int for index variables to make sure you don't invade the out of bounds exception land. (assigning -ve values to such variables leads to a warning which is fairly easy to decode).

Two problems I can see:

• Your memory allocation is not proper. Each of your rows share a common vector variable because of the stmt:

vector<int> v(n);
vector<vector<int> > data(n,v);

Here the memory for each inner row is getting allocated only once and your two dimensional matrix gets populated with the same vector.

You need to change it to:

vector<vector<int> > data(n,vector<int> (n));

So that the memory allocation occurs for each row rather than only once for all rows.

• Your logic is skewed. The line :

i=0;
    j=n/2;
    for (int k=1; k<=nsqr; ++k)
    {
        data[i][j] = k;
        i--;

is bound to produce an out of bounds error since you are decemeneting i which already is zero, resulting in its value becoming -1, which is not a valid index.

Better use size_t instead of int for index variables to make sure you don't invade the out of bounds exception land. (assigning -ve values to such variables leads to a warning which is fairly easy to decode).

Ok, thanks a lot man. I've finally fixed my code, and the proram works fine now. Thanks for all the help :)
Cheers!

This question has already been answered. Start a new discussion instead.