Jump to content
  • Advertisement
Sign in to follow this  
KieranW

C++ - Debug Assertion Error when Class isn't declared on the heap

This topic is 3610 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Ok I have a class named Mystic which is derived from a class named Combatant, and when I declare an instance of Mystic like such: Mystic player; And I run the code, I get a debug assertion error which means I have in some way corrupted the heap. However when I declare it like this: Mystic* player = new Mystic(); I no longer get the debug assertion error. Now what I would like to know is if this is normal and classes like this should be created on the heap, or if there is a problem with my code that should be fixed. Here is the code for Combatant and Mystic. Combatant.h:
#ifndef _MYSTICS_COMBATANT_H				//if the class isn't already defined
#define _MYSTICS_COMBATANT_H				//then define and create it

#include "Effect.h"

class Combatant
{

public:

	Combatant(void);						//class constructor
	~Combatant(void);						//class destructor

	virtual bool Initialize(char* name,		//class initializer
					int level,		
					int health);

	char* GetName(void);					//gets the combatants name
	int GetLevel(void);						//gets the combatants level
	int GetHealth(bool current);			//gets the combatants health
	int GetEffectsAmount(void);				//gets the number of effects

protected:

	char* m_pName;							//name for the combatant
	int* m_pLevel;							//level for the combatant
	int* m_pHealth;							//array for current/max health
	Effect** m_pEffects;					//array that holds effects on the combatant
	int* m_pEffectsAmount;					//holds the amount of effects on combatant

};

#endif
Combatant.cpp
//----- Class Combatant -----//

#include "Combatant.h"
#include <string>

Combatant::Combatant(void)
{
	//empty
}

Combatant::~Combatant(void)
{
	delete [] m_pName;
	delete m_pLevel;
	delete [] m_pHealth;
}

bool Combatant::Initialize(char* name, int level, int health)
{
	m_pName = new char[strlen(name) + 1];
	for(int i = 0; i < strlen(name); i++)
	{
		m_pName = name;
	}
	m_pName[strlen(name)] = NULL;

	m_pLevel = new int(level);
	m_pHealth = new int[2];
	m_pHealth[0] = health;
	m_pHealth[1] = health;

	return true;
}

char* Combatant::GetName(void)
{
	return m_pName;
}

int Combatant::GetLevel(void)
{
	return *m_pLevel;
}

int Combatant::GetHealth(bool current)
{
	return m_pHealth[current];
}

int Combatant::GetEffectsAmount(void)
{
	return *m_pEffectsAmount;
}
Mystic.h
#ifndef _MYSTICS_MYSTIC_H					//if the class isn't already defined
#define _MYSTICS_MYSTIC_H					//then define and create it

#include "Combatant.h"
#include "Spell.h"

class Mystic : public Combatant
{

public:

	Mystic(void);							//class constructor
	~Mystic(void);							//class destructor

	bool Initialize(char* name,				//class initializer
					char* element1,
					char* element2,
					int level,
					int experience,
					int experienceNeeded,
					int health,
					int energy,
					int fireResistance,
					int waterResistance,
					int windResistance,
					int rockResistance);

	char* GetElement(int element);
	int GetExperience(bool current);
	int GetEnergy(bool current);
	int GetResistance(int resistance);
	int GetSpellsAmount(void);
	static int GetInstanceCount(void);

private:

	static int s_Instances;					//holds the number of mystics created
	char** m_pElements;						//array to store elements
	int* m_pExperience;						//array to store current/max experience
	int* m_pEnergy;							//array to store current/max energy
	int* m_pResistances;					//array to store resistances
	Spell** m_pSpells;						//array to store spells
	int* m_pSpellsAmount;					//holds the amount of spells

};

#endif
Mystic.cpp
#include "Mystic.h"
#include <string>

int Mystic::s_Instances = 0;

Mystic::Mystic(void): Combatant()
{
	s_Instances++;
}

Mystic::~Mystic(void)
{
	delete [] m_pName;
	delete [] m_pElements;
	delete m_pLevel;
	delete [] m_pExperience;
	delete [] m_pHealth;
	delete [] m_pEnergy;
	delete [] m_pResistances;
}

bool Mystic::Initialize(char* name, char* element1, char* element2, int level, int experience, int experienceNeeded, int health,
						int energy, int fireResistance, int waterResistance, int windResistance, int rockResistance)
{
	m_pName = new char[strlen(name) + 1];
	for(int i = 0; i < strlen(name); i++)
	{
		m_pName = name;
	}
	m_pName[strlen(name)] = NULL;

	m_pElements = new char*[3];
	m_pElements[0] = element1;
	m_pElements[1] = element2;
	m_pElements[2] = NULL;

	m_pLevel = new int(level);
	m_pExperience = new int[2];
	m_pExperience[0] = experience;
	m_pExperience[1] = experienceNeeded;
	m_pHealth = new int[2];
	m_pHealth[0] = health;
	m_pHealth[1] = health;
	m_pEnergy = new int[2];
	m_pEnergy[0] = energy;
	m_pEnergy[1] = energy;
	m_pResistances = new int[4];
	m_pResistances[0] = fireResistance;
	m_pResistances[1] = waterResistance;
	m_pResistances[2] = windResistance;
	m_pResistances[3] = rockResistance;

	return true;
}

char* Mystic::GetElement(int element)
{
	return m_pElements[element];
}

int Mystic::GetExperience(bool current)
{
	return m_pExperience[current];
}

int Mystic::GetResistance(int resistance)
{
	return m_pResistances[resistance];
}

int Mystic::GetSpellsAmount(void)
{
	return *m_pSpellsAmount;
}

int Mystic::GetInstanceCount(void)
{
	return s_Instances;
}

Share this post


Link to post
Share on other sites
Advertisement
The destructor for Mystic deletes m_pLevel, but so does the destructor Combatant, this causes a double delete. Since Mystic inherits Combatant the compiler automatically knows to call Combatant's destructor after calling Mystic's destructor. I think the reason using new fixed this is that you never deleted the object so it was never destructed.

There may be other problems, but this was the most obvious one.

Several general hints too (please don't take offence, i'm trying to help you write better code):
* Use std::string. It's a lot simpler than using char* and a lot safer.
* I'm not sure if you added those comments just for posting here, but they really are unnecessary. It is obvious from the method name GetName what the method will do. You only need a comment if what the method does is not obvious.
* Also consider using std::vector instead of your array of Effects

Share this post


Link to post
Share on other sites
Thanks for moving the thread.

Wow I feel stupid, I can't believe I didn't see that. I also redeleted m_pName, m_pLevel and m_pHealth not just level XD.

No no, offence not taken at all, in fact that's the kind of help that I need. The comments were for me, but yes I agree they are unneccesary and I will remove them.

As for std::string I was using that but I changed to char* because win32 only took LPCWSTR for some of the commands and visual c++ 6.0 was able to convert char* to LPCWSTR but not std::string.

And as for std::vector I do know how to use them but I don't see how they are an advantage over arrays, and also I heard they take up more memory(however I am not positive).

Share this post


Link to post
Share on other sites
Also, since you're using a virtual function (and thus probably store pointers to Combatants) you should make the destructor of Combatant virtual.

If you're ever going to delete a derived class by calling delete on a base pointer type the destructor has to be virtual. Otherwise you'll end up with shearing (destroying only the base part of an object).

Share this post


Link to post
Share on other sites
Quote:
And as for std::vector I do know how to use them but I don't see how they are an advantage over arrays, and also I heard they take up more memory(however I am not positive).


Std::vector has an overhead of about 8 bytes compared to an equal sized array. It's much safer - if you access an item beyond its boundaries it doesn't mangle your heap but either asserts or enlarges the vector.

If you don't want the enlarging behaviour, look around for an array wrapper - they're exactly the same size as the array but don't corrupt the heap if you misaccess them.

Why all the pointers?

Quote:
As for std::string I was using that but I changed to char* because win32 only took LPCWSTR for some of the commands and visual c++ 6.0 was able to convert char* to LPCWSTR but not std::string.


Why in gods name do you use a compiler that's more than ten years old, when the 3 generations newer one is free for all sorts of use? Try Visual C++ 2008 express.

Also, if you want a char *, try putting .c_str() behind the std::string when you pass it to a function that doesn't accept std::strings.

The compiler shouldn't convert from char * to LPCWSTR automatically - they're not compatible. A small conversion table for the details:

LPCSTR: const char *
LPCWSTR: const wchar_t *
LPCTSTR: const <whatever char type you selected in project options> *

I suspect it's actually an LPCTSTR which accidentally works. If you need a LPCWSTR, try std::wstring and it's .c_str(). If you want to do it entirely right, use this line in a header defining the string type:


namespace std { typedef basic_string<TCHAR> tstring; }


You need to include windows.h (for TCHAR) and string (for std::basic_string) before this line.

This defines a new type tstring, which is always equal to what LPCTSTR would be and allows you to work with it in every place that takes an LPCTSTR.

Share this post


Link to post
Share on other sites
Ah... whatever. Here's an array class:


template <typename T, int n>
class array {
private:
T items[n];
public:
array() {}
~array() {}
T &operator[](int index)
{
assert(index >= 0 && index < n);
return items[index];
}
};

Use as

int m_Magic[2] => array<int, 2> m_Magic

Share this post


Link to post
Share on other sites
If you know something is going to be allocated as an array of say 2 ints, just make it a normal array:


int m_pExperience[2];
int m_pHealth[2];
int m_pEnergy[2];
int m_pResistance[4];
//better have an enum of resistance types, so you can do:
//int m_pResistance[max_resistance_types];



Don't allocate things dynamically if you don't have to. For arrays that you need to allocate dynamically (because of unknown runtime size) use appropriate container, std::string for string data, std::vector for dynamic arrays etc.

Quote:

Std::vector has an overhead of about 8 bytes compared to an equal sized array. It's much safer - if you access an item beyond its boundaries it doesn't mangle your heap but either asserts or enlarges the vector.


Well, it lets you mess up the heap alright if you access items beyond boundaries (VC++ by default checks all accesses at runtime but other compilers' implementations needn't do that), but it is easier to stay in bounds since the vector knows its size at any time, and of course it takes care of any dynamic memory management, including making it easy to enlarge the array. It is easy to use it in safe ways.

Share this post


Link to post
Share on other sites
Quote:
Why in gods name do you use a compiler that's more than ten years old, when the 3 generations newer one is free for all sorts of use? Try Visual C++ 2008 express.


Visual C++ 6.0 is actually the best compiler I have used out of all the visual studios and dev-c++. I tried Visual C++ 2008 express and it gave me errors in code that worked fine in 6.0, it didn't let me use resources, and the environment wasn't as nice. Dev-C++, didn't help with errors as nicely, not very nice syntax highlighting and I don't like the environment as much. After the service packs for 6.0 it is actually a very nice compiler.

And yes you are right, it was LPCTSTR not LPCWSTR, sorry about the confusion.

Share this post


Link to post
Share on other sites
Quote:
Original post by KieranW
Quote:
Why in gods name do you use a compiler that's more than ten years old, when the 3 generations newer one is free for all sorts of use? Try Visual C++ 2008 express.


Visual C++ 6.0 is actually the best compiler I have used out of all the visual studios and dev-c++. I tried Visual C++ 2008 express and it gave me errors in code that worked fine in 6.0, it didn't let me use resources, and the environment wasn't as nice. Dev-C++, didn't help with errors as nicely, not very nice syntax highlighting and I don't like the environment as much. After the service packs for 6.0 it is actually a very nice compiler.


Some might disagree. Usually any code that won't compile in VC2008 that did compile in VC 6 indicates that the problem is with your code. VC 6 predates the standardisation of C++. VC 2008 is quite good at following the standard.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!