Advertisement

My First Class!!

Started by August 09, 2002 02:28 PM
12 comments, last by algumacoisaqualquer 22 years, 4 months ago
This is the first time i have used a class structure on a real project. It works fine while the program is running, but I get an ilegal error when I close the program. First of all, i have no idea of how the destructor work, so i made one but I never call him. Anyway, i'm going to post the class here, i would apreciate any comments you have:
  

////////////////

//The header: //

////////////////


#ifndef CPK_MAP_CLASS_H
#define CPK_MAP_CLASS_H

class LevelMap {

// It's a map class, it stores the info of the map

// that is displayed in the screen right now (the ScreenMap

// array) and the info of the World map (the Map array).

// SizeX, SizeY are the sizes of the world map,

// ScreenX, ScreenY are the number of tiles in the ScreenMap

// MapX, MapY are the coordinates of where the ScreenMap is

// in relation with the world Map, So ScreenMap(0,0) stores

// the value of Map(MapX, MapY).


public:
char *ScreenMap;
LevelMap();
LevelMap(int map_x, int map_y,
         int screen_x, int screen_y);
~LevelMap();


char GetAbsoluteTile(int x, int y);
char GetTile(int x, int y);
bool LevelMap::CreateMap();
bool LevelMap::SetTile(int x, int y, char Value); 
bool LevelMap::UpdateMap();              
void LevelMap::ClearMap();               
void LevelMap::UpdateScreen(); 
bool LevelMap::SetPosition(int AtX, int AtY);  

private:
int MapX,               
    MapY;    
int SizeX, SizeY;
int ScreenX, ScreenY;
char *Map;             
};

#endif

///////////////////////////

//And Now the Cpp code:  //

///////////////////////////


#include "CPKMapClass.h"

LevelMap::LevelMap(int map_x, int map_y, int screen_x, int screen_y)
{
  SizeX = map_x;
  SizeY = map_y;
  ScreenX = screen_x;
  ScreenY = screen_y;
  CreateMap();

}
LevelMap::LevelMap()
{
  SizeX = 60;
  SizeY = 30;
  ScreenX = 11;
  ScreenY = 8;
  CreateMap();

}
LevelMap::~LevelMap()
{

  delete ScreenMap;
  delete Map;

}

  char LevelMap::GetAbsoluteTile(int x, int y)
{
  return Map[(x*ScreenY) + y];
}

  char LevelMap::GetTile(int x, int y)
{
  return ScreenMap[(x*ScreenY) + y];
}

bool LevelMap::CreateMap()
{
  int MapSize;
  MapSize = SizeX*SizeY;
  Map = new char[MapSize];
  MapX = 0;
  MapY = 0;
  MapSize = ScreenX*ScreenY;
  ScreenMap = new char[MapSize];
  return true;
}
bool LevelMap::SetTile(int x, int y, char Value)
{
ScreenMap[(x*ScreenY) + y] = Value;
return UpdateMap();
}
bool LevelMap::UpdateMap()
{
 return true;
}
void LevelMap::ClearMap()
{
char *MapPoint;
MapPoint = Map;
int i;
  for(i = 0; i <=SizeX*SizeY; i++)
  {
    *MapPoint = 55;
    MapPoint++;
  }
}

void LevelMap::UpdateScreen()
{
int i,j, indice;
char Value;
for(i = 0; i < ScreenX; i++)
   {for(j = 0; j < ScreenY; j++)
      {
      indice = (i + MapX)*SizeY + (j + MapY);
      Value = Map[indice];
      indice = (i * ScreenY) + j;
      ScreenMap[indice] = Value;
      }
   }
}

bool LevelMap::SetPosition(int AtX, int AtY)
{
if(AtX <= (SizeX - ScreenX) && AtY <= (SizeY - ScreenY))
    {
    MapX = AtX;
    MapY = AtY;
    return true;
    }
else
    {MapX = 0;
    MapY = 0;
    return false;
    }
}
////////////////////////////////////////////////////////////////

// This is the part of the main that interacts with the class //

////////////////////////////////////////////////////////////////



LevelMap CenMap(80, 40, 10, 8); // declared as global, not inside any function.


int InitGL(GLvoid)   //yes, it's a NeHe based							

{
//(...)

    CenMap.ClearMap();
//(...)

}

int draw_tiles(GLvoid)
{
int tile;
    for (int y = 0; y < 9; y++)
    { for (int x = 0; x < 7; x++)
     {
     CenMap.UpdateScreen();
     tile = CenMap.GetTile(x, y);
      if (tile != 0)
         {
         //(DrawStuff)

         }
//(...)

}
   
PS. sorry about the lack of comments, but that's because they were not in English. Thanks! [edited by - algumacoisaqualquer on August 9, 2002 3:37:02 PM]
Change your deconstructor to:


    LevelMap::~LevelMap(){  delete [] ScreenMap;  delete [] Map;}    


[EDIT] Perhaps I should explain myself... You need the "[]" if and only if it is a dynamic array (doing so when you only allocated a new pointer will also give you errors)

_____________________________

And the Phoenix shall rise from the ashes...

--Thunder_Hawk -- ¦þ
______________________________

[edited by - Thunder_Hawk on August 9, 2002 3:46:05 PM]
______________________________________________________________________________________The Phoenix shall arise from the ashes... ThunderHawk -- ¦þ"So. Any n00bs need some pointers? I have a std::vector<n00b*> right here..." - ZahlmanMySite | Forum FAQ | File Formats______________________________________________________________________________________
Advertisement
Thunder_Hawk is right, there may be something else causing the problem. ScreenMap and Map are pointers but there is the possibility that they can be deleted by the destructor without ever being initialized because CreateMap has to be called to assign them with new. I think "delete"ing a pointer that was never "new"ed can cause a problem like you are having.
Also, your question about the destructor. You don''t call it explicitly. It gets called automatically when you "delete" the object if it is a pointer, or when the object loses scope if it is not a pointer.
I did that, but it still gives me the same error...
Should I also delete every variable I created? Do I have to call the destructor or it is automatic or something?

Anyway... Thanks for the help!

[EDIT] - Sorry I hadn't seen the AP post. Well, i call the CreateMap at both my constructors, shouldn't my map arrays be always created? Should I place the CreateMap part directly in the constructor?

[EDIT AGAIN] - I just placed the |ScreenMap = new char[MapSize];| part in the constructor, and I get the same error. Also, in my program, the tiles are being drawed with the correct value, so the program is reading the arrays, I think.

[edited by - algumacoisaqualquer on August 9, 2002 4:05:49 PM]

[edited by - algumacoisaqualquer on August 9, 2002 4:09:05 PM]
quote: Original post by Anonymous Poster
Thunder_Hawk is right, there may be something else causing the problem. ScreenMap and Map are pointers but there is the possibility that they can be deleted by the destructor without ever being initialized because CreateMap has to be called to assign them with new. I think "delete"ing a pointer that was never "new"ed can cause a problem like you are having.
Also, your question about the destructor. You don''t call it explicitly. It gets called automatically when you "delete" the object if it is a pointer, or when the object loses scope if it is not a pointer.


Both of his constructors call createmap so his pointers should always be initialized. Also, considering that he gets the problems when he closes the program, its seems obvious that the object is global and the deconstructor is messed up. While I''m still talking about this, that deconstructor is quite neccessary or your program will eat up the system resources each time it runs. Good job writing your first class!



_____________________________

And the Phoenix shall rise from the ashes...

--Thunder_Hawk -- ¦þ
______________________________
______________________________________________________________________________________The Phoenix shall arise from the ashes... ThunderHawk -- ¦þ"So. Any n00bs need some pointers? I have a std::vector<n00b*> right here..." - ZahlmanMySite | Forum FAQ | File Formats______________________________________________________________________________________

  if(map){   delete map; // or delete[]}  


if you don''t initialize a pointer(set it equal to NULL or something) and try and delete it it crashes your program.
this little code fragment checks to see if something in in map and if so delete.

and destructors are called automatically where you program ends, unless the class in declared with new.)

"It's not that it works, but how and why."
Advertisement
Ah, I missed that part. Hmmm well that makes it a bit more interesting. I guess we would have to see more code. It could be something not related to the class. Have you tried running the program through the debugger and see where the error is occuring and trace through the call stack? Other than that, not sure. Does CreateMap() get called more than once, like outside of the constructor somewhere?
Thunder_Hawk - Well... Thanks!

dead_man_walking - sorry, but that didn''t made it, is getting the same error. I didn''t get the error with if (!ScreenMap){delete []ScreenMap;}, but I thinks this isn''t releasing any memory, so it is kind of pointless.

AP - I think that the problem is in the constructor, because it closed fine when I placed the delete part on coments, it has to be something there. i can post the code here, but i think it will be a little dificult to follow it, plus, I wasn''t geting any errors before using this class.

Thanks all you guys for the help!
PROBLEM SOLVED!!!!

Well, it turns out the problem was in the ClearMap method (at least it doesen''t give the ilegal operation thing anymore)...


  //WRONG!!void LevelMap::ClearMap(){char *MapPoint;MapPoint = Map;int i;  for(i = 0; i <=SizeX*SizeY; i++)  {    *MapPoint = 55;    MapPoint++;  }}//RIGHT!!void LevelMap::ClearMap(){char *MapPoint;MapPoint = Map;int i;  for(i = 0; i <SizeX*SizeY; i++)  {    *MapPoint = 55;    MapPoint++;  }}  


The diference is in the i <=SizeX*SizeY, this way, he will give a value to MapPoint[SizeX*SizeY + 1], wich does not exist. Well, I don''t know if that''s the problem, but it certainly wasn''t right, and after i fixed it, no more problems happend.

Well, thank all of you, even with the problem somewhere else, this helped me to understand classes and destructors a lot better (and had the delete []Map problem fixed). Thanks!
Different AP:
Dead_man_walking is on to something. Try initializing your pointers with NULL in the constructor, then testing to see if they are null before attempting to delete them in the deconstructor.

Classes don''t initialize values at zero. You have to do that in the constructor.

So it''s:
ScreenMap = NULL;Map = NULL 

In the Constructor, then

if(Map != NULL) delete [] Map;if(ScreenMap != NULL) delete [] ScreenMap; 


in the destructor.

-James

This topic is closed to new replies.

Advertisement