Program keeps crashing

Started by
3 comments, last by Trienco 12 years, 2 months ago

#include<iostream>
#include<string>
using namespace std;

class enemy
{
private:
int m_health; //the health of each enemy
string m_name; //name of enemy jumping you
enemy* pNext; //points to next object on the list

public:
void taunt(); //taunts the user
void setNext(enemy* next); //gives next enemy on the list
enemy* getNext() const; //returns next enemy on the battlefield
string getName(); //returns the enemies name
enemy(string name = " ", int health = 100);
};

enemy::enemy(string name, int health):
m_health(health),
m_name(name),
pNext(0)
{}

void enemy::setNext(enemy* next)
{
pNext = next;
}

enemy* enemy::getNext() const
{
return pNext;
}

void enemy::taunt()
{
cout << "Ha Ha You suck, you dumb loser !" << endl;
}

string enemy::getName()
{
return m_name;
}

class battlefield
{
private:
enemy* m_pHead; // the top of the chain

public:
void AddEnemy();
void DeleteEnemy();
void EnemyTaunt();
void Clear();
void GetEnemiesName(); //list through all the names of the enemies made
void searchName(string* enemyName);
void killEnemy(); //decides which enemy to kill

battlefield();
~battlefield();
};

battlefield::battlefield():
m_pHead(0)
{}

battlefield::~battlefield()
{
cout << "Clearing the BATTLEFIELD !" << endl;

Clear();
}

void battlefield::Clear()
{
while(m_pHead != 0)
{
DeleteEnemy();
}
}

void battlefield::AddEnemy()
{
cout << "What type of enemy is this: ";
string enemyType;
cin >> enemyType;

enemy* pNewEnemy = new enemy(enemyType);

if(m_pHead == 0)
{
m_pHead = pNewEnemy;
}

else
{
enemy* pIter = m_pHead;
while(pIter->getNext() != 0)
{
pIter = pIter->getNext();
}
pIter->setNext(pNewEnemy);
}
}

void battlefield::DeleteEnemy()
{
if(m_pHead == 0)
{
cout << "The garage is empty !" << endl;
}
else
{
enemy* pTemp = m_pHead;
m_pHead = m_pHead->getNext();
delete pTemp;
}
}


void battlefield::GetEnemiesName()
{
enemy* pFirstEnemy = m_pHead;

cout << pFirstEnemy->getName() << endl;

while(pFirstEnemy->getNext() != 0)
{
pFirstEnemy = pFirstEnemy->getNext(); // gets the next object
cout << pFirstEnemy->getName() << endl; // returns name

}
}

void battlefield::searchName(string* enemyName)
{
enemy* searchEnemy = m_pHead;
string p_EnemyName;

if( searchEnemy->getName() == *enemyName)
{
cout << "We've found the enemy, He's FIRST !" << endl;
}

//iterating through list of objects
while(searchEnemy->getNext() != 0)
{
searchEnemy = searchEnemy->getNext(); //gets next pointer
p_EnemyName = searchEnemy->getName(); // gets the name

if( *enemyName == p_EnemyName )
{
cout << "We found the enemy !" << endl;
}
}
}

void battlefield::killEnemy()
{
cout << "Type in the name of the enemy you would like to kill: ";
string enemyName;
cin >> enemyName;

enemy* p_EnemyKill = m_pHead;
enemy* pEnemyName;

while(p_EnemyKill->getNext() != 0)
{
p_EnemyKill = p_EnemyKill->getNext();
*pEnemyName = p_EnemyKill->getName();
}

delete pEnemyName;
pEnemyName = 0;
}


void battlefield::EnemyTaunt()
{
enemy* tauntEnemy = m_pHead; // the head enemy will taunt.

cout << "The enemy is taunting you.\n";

tauntEnemy->taunt();
}

int main()
{
battlefield theBattleField;

cout << "Enemies are spawning !" << endl;
int number;
cout << "Please enter a number: ";
cin >> number;

for(int i = 0; i < number; i += 5)
{
theBattleField.AddEnemy();
}

theBattleField.EnemyTaunt();
theBattleField.GetEnemiesName();

cout << "Type in the name of the enemy you would like to search: ";
string enemySearch;
cin >> enemySearch;

theBattleField.searchName(&enemySearch);

theBattleField.killEnemy();

theBattleField.GetEnemiesName();

return 0;

}


I made this game, where the user enters a number and makes a bunch of enemies based on that number. What I wanted to try was calling a function that asks a user to type in the enemy name, and will kill that enemy. everytime i run the game crashes. Any Ideas on why ?
Advertisement
[font=courier new,courier,monospace]Are you sure it's crashing? Maybe the console window is closing after the program has finished? It doesn't crash for me when I run it from a command line. What happens if you add some code to have the user 'press any key' before exitting the program?[/font]

[font=courier new,courier,monospace]Additionally, why is i += 5 in the following loop? Is the number I am requested to enter supposed to equal the number of enemies? When I enter '2' or '3' I am only ever asked to enter one enemy type.[/font]

for(int i = 0; i < number; i += 5)
{
theBattleField.AddEnemy();
}
For every number divisible by 5, so 10 equal 2 enemies.
I running it in codeblocks and it crashes
There are plenty of ways to crash, the most obvious being that you just go and delete an enemy (always the LAST one, no matter which name was entered) and don't seem to bother removing him from the list. Even worse, you don't even delete the enemy at all. This code shouldn't even compile, because you assign a string to a enemy*. Even if it was the correct pointer and you were deleting the enemy, the last line (which should probably be)
p_EnemyKill = 0;


is essentially pointless. pEnemyKill is a local variable, setting it to 0 before the end of the function doesn't do anything and most of all it won't affect ANY of the other pointers (especially the next pointer of the previous enemy). Instead of going on about copies and stuff, I'll just make a simple example:

int a = 5;
int b = a;
b = 6;

Is a now 6, just because b was set to the _value_ of b at first? Of course not.

Removing an entry from the middle of the list requires doing a lot more than that. You need to keep a pointer to the previous element so you can make it point to the element after the one to delete. There are also ways to avoid the special handling of head (which doesn't have a previous entry), but they involve a pointer to pointer and I would stay away from that for now.

There are also examples of code that is needlessly complex and could be shorter, cleaner AND more correct/robust.


enemy* pFirstEnemy = m_pHead;

cout << pFirstEnemy->getName() << endl;

while(pFirstEnemy->getNext() != 0)
{
pFirstEnemy = pFirstEnemy->getNext(); // gets the next object
cout << pFirstEnemy->getName() << endl; // returns name

}



I notice at least two functions like that and you never check if the list is empty and a first element even exists. Once you killed the last enemy, this would crash. There is also no good reason to handle the first element separately like it was special in any way.



enemy* pEnemy = m_pHead;

while(pEnemy)
{
cout << pEnemy->getName() << endl; // returns name
pEnemy = pEnemy->getNext(); // gets the next object
}

I also assume that this is your "list project" where you play with linked lists to understand how they work.. and then never ever do that again when you could just use std::list<Enemy> . Especially since muddling the class with list pointers might be the simple way for this example, but in a "real" class:

a) why would an object care if it is going to be stored in a list? (there might be exceptions, for example for pathfinding, though it would be more likely a prev pointer)
b) what if you decide the object needs to be in more than one list at a time?
f@dzhttp://festini.device-zero.de

This topic is closed to new replies.

Advertisement