problem with std::list

Started by
14 comments, last by graveyard filla 19 years, 10 months ago
quote:
Just trying to get things straight in my head here. So you need a proper copy constructor to work with std::vector and std::list incase your class contains pointers. What is the syntax to a copy constructor?

Is it for example:
CVector2 NewVec(5.0f, 2.0f);

or:
CVector2 NewVec;
NewVec.x = 5.0f;
NewVec.y = 2.0f;
CVector2 NewVecTwo = NewVec;


Is it a constructor that accepts paramters or a constructor that gets called when you write down that last line in the 2nd example?


You have to check STL's documentation for type requirements because it's not the same for every container, vector has only one possiable requirement which is assignment operator to be defined but you'll only need to write one if you have says pointers as members if you don't wont pointers to be just copied and referring to the same object etc.

Using your CVector2 class as an example i'll try an answer your questions.

A type's copy constructor is a function constructor that takes a constant reference parameter of it's type e.g.

 CVector2(const CVector2& v): x(v.x), y(v.y) {}        


A type's default constructor is a function constructor that doesn't need any arguments passed to it for initialization, two ways to declare one:

1. One that has no arguments
 CVector2(): x(0), y(0) {}       


2. One that provides default values for all it's arguments, that allows you to achieve the same as above
 CVector2(float nx = 0, float nx = 0): x(nx), y(ny) {}       


A type's assignment operator is defined as follows
 CVector2& operator=(const CVector2&);     


so we how it's defined, this is when there called:
 int main() {     CVector2 v; //<--- default constructor called     CVector2 v2(3, 3);     CVector2 v3(v2); //<-- copy constructor called     CVector2 v4 = v3; //<-- assignment operator called     return 0;}


quote:
Original post by graveyard filla
its still crashing...ive never made a copy constructor or done operator overloading before. could you explain what exactly it should all look like? my class has a member which is a pointer that i dynamically allocate (which i think is why it suddenly started crashing - i made this member, and now this is happening).. i just dont understand what my CC / = operator should look like. this is what i got so far:

(also, when is my CC called? i have to do something like Enemy enemy2 = new Enemy(enemy1); ? )

also, i couldnt find the answer to this : what happends when my = operator is called? say i do Enemy newenemy = enemy1; what does this do? it calls my = operator, and sends enemy1 as paremters? and newenemy is assigned whatever is returned?

                      class Enemy : public Character{	public:		Enemy();		Enemy(const Enemy& cpy);		~Enemy(){//SDL_FreeSurface(health_img);//SDL_FreeSurface(dead_img);		        }		Enemy operator = (Enemy cpy);				virtual void Init();		virtual void Update();			//handle input, collision detection, ...				void Render();		void Draw_Health();		bool On_Screen();		int Clicks_Away();	private:					GLuint health_img;	        Sprite sprite;	        GLuint texture;};now in the cpp file..   Enemy::Enemy(const Enemy &cpy){	xPos = cpy.xPos;	yPos = cpy.yPos;	xVel = cpy.xVel;	yVel = cpy.yVel;	h = PLAYERHEIGHT;	//save player height and width	w = PLAYERWIDTH;	xVel = 0;			//speed	yVel = 0;	current_weapon = cpy.current_weapon;	health_img = cpy.health_img;	texture = cpy.texture;	state = cpy.state;	sprite = cpy.sprite;	current_target = cpy.current_target;	ammo = cpy.ammo;	health = cpy.health;	max_health = cpy.max_health;}Enemy Enemy::operator = (Enemy cpy) {    Enemy temp;  	temp.xPos = cpy.xPos;	temp.yPos = cpy.yPos;	temp.xVel = cpy.xVel;	temp.yVel = cpy.yVel;	temp.h = PLAYERHEIGHT;	//save player height and width	temp.w = PLAYERWIDTH;	temp.xVel = 0;			//speed	temp.yVel = 0;	temp.current_weapon = cpy.current_weapon;	temp.health_img = cpy.health_img;	temp.texture = cpy.texture;	temp.state = cpy.state;	temp.sprite = cpy.sprite;	temp.current_target = cpy.current_target;	temp.ammo = cpy.ammo;	temp.health = cpy.health;	temp.max_health = cpy.max_health;    return (temp);}


current_weapon is a pointer... a Weapons* pointer which i dynamically allocate anytime the character switches weapons (this *current_weapon is a member of Character which Enemy inherits from.. maybe i should make a copy constructor for Character instead?) i think i should be doing something with this current_weapon, altho im just doing current_weapon = cpy.current_weapon... thanks for any help!

[edited by - graveyard filla on June 6, 2004 2:10:15 PM]


Yes your most probably getting problems with members that are pointers, you should post your Character class. Your assignment operator isn't correct, it is valid but its not a proper assignment operator. Without seeing your Character class its hard to know exactly where the problem lies, we need to know if its a concreate/abstract base class etc.

I'll assume it's an abstract class for the time being, yes it would be better if you defined a protected constructor so you can initialize things properly and a virtual deconstructor so sub-types can override the default if they need to. If your dynamically allocating there you should deallocate there aswell there are exceptions to this of course buts its another story.

Does it make sense to provide a copy constructor for an abstract type i dont, i think concreate types should handle it, there representation is complete so to say.

Also try to avoid putting data members in a base class as protected, make them private and provide protected inline member functions for dervied types to use instead of directly access them. You should have it something like this for the base:
             class Character {  int* int_ptr;  protected:  Character(int* ptr = 0): int_ptr(ptr) {     if(int_ptr == 0)        int_ptr = new int(3); //just an example  }    const int* get_int_ptr() const {       return int_ptr;  }  int* get_int_ptr() {       return int_ptr;  } public:  //pure virtual makes it an ABC class  virtual void draw() const = 0;   virtual ~Character() {      if(int_ptr != 0)         delete int_ptr;  }};


Now for a the Enemy concreate type with the wright default, deconstructor, copy and assignment operators:

        class Enemy : public Character{	public:		   Enemy(int* ptr = 0): Character(ptr) {} //default constructor   Enemy(const Enemy& e): Character(), //call default base                             //add other member init list here   {}   Enemy& operator=(const Enemy& e) { //assignment operator      return *this;   }   void draw() const;   ~Enemy();//<-- override base deconstructor if you like};


[edited by - snk_kid on June 6, 2004 3:52:49 PM]
Advertisement
sorry im still a little confused... what should i do then to ensure that everything is copied correctly and nothing is deleted /allocated that shouldnt have to? heres my Character class... and yes i never instantiate it.. only instantiate the classes which derive from Character... also, i DO free the pointer in the Character destructor... heres it is..

class Character: public Entity{		public:		Character(){}			//delete our current weapon if its not NULL		virtual ~Character(){if(current_weapon) delete current_weapon;}		virtual void Init() = 0;		virtual void Update() = 0;			void Give_Damage(Character *target);		void Take_Damage(const Damage &dmg);		float Get_Health_Percent(){return (float)health/(float)max_health;}		void Collision_Detection_Map();								//does the whole collision detection		bool Collision_Vert(int x, int y, int &tilecoordx);			//tests for collision with a tile on the vertikal line from [x,y] to [x,y+height]		bool Collision_Horz(int x, int y, int &tilecoordy);		//horizontal line from [x,y] to [x+width, y]					bool Is_Dead(){return health <= 0;}	protected:		Character *current_target;		Weapons   *current_weapon;				int ammo;		int health;		int max_health;				Character_States state;			 };


like i said, current_weapon is the pointer which im allocating...
FTA, my 2D futuristic action MMORPG
Your not initializing your members and member pointers to either 0 or the address of some object so its starts with some garbage value for an address, in your deconstructor your if statement is flagging true and trying to delete some memory somewhere and bang!

What i would first do is provide a constructor that takes pointer arguments and provides default values of zero for all of them to define a default constructor. Then Enermy will call the base constructor in its constructor to make sure its all initialized.

Also its really bad to make data members protected access, you'll end up with tight coupling between sub-types, change it to private and provide protected inline member functions for derived classes if there isn't any public accessor functions already. Also if you have non state modifying operations like is_Dead(), make it a constant member function.

[edited by - snk_kid on June 6, 2004 5:34:07 PM]
i dunno but who cares if its just one pointer you have to shuffle around?

you odviosly(obviosly? i dunno) changed the initialization of your enemy container somehow,

but who cares?

mabey ill read and find out
quote:Original post by snk_kid
Your not initializing your members and member pointers to either 0 or the address of some object so its starts with some garbage value for an address, in your deconstructor your if statement is flagging true and trying to delete some memory somewhere and bang!

What i would first do is provide a constructor that takes pointer arguments and provides default values of zero for all of them to define a default constructor. Then Enermy will call the base constructor in its constructor to make sure its all initialized.

Also its really bad to make data members protected access, you'll end up with tight coupling between sub-types, change it to private and provide protected inline member functions for derived classes if there isn't any public accessor functions already. Also if you have non state modifying operations like is_Dead(), make it a constant member function.

[edited by - snk_kid on June 6, 2004 5:34:07 PM]


doh.. all i had to do was add a current_weapon = NULL; line to my character constructor, and that fixed everything! so i never even needed a copy constructor / equal overload (i commented them out, and everything is good..)

could you please explain why its bad to have protected members? and you think i should make them private, then in the base class make Get() and Set() functions? i thought if i made a variable private in a base, anyone that inherited from the class wouldnt inherit the private members... and why inline? and why make Is_Dead const? thanks a lot for all your help!!!

[edited by - graveyard filla on June 6, 2004 6:03:23 PM]
FTA, my 2D futuristic action MMORPG
Getting back to the original problem:

If i was you, i''d just shuffle pointers around. It''s going to get more and more innefficient the more objects you have if you are passing the objects themselves.

This topic is closed to new replies.

Advertisement