Archived

This topic is now archived and is closed to further replies.

Gregor_Samsa

What's wrong with this?

Recommended Posts

What''s wrong with this code? #include //#include "List.cpp" //using namespace std; //introduces namespace std class Bullet { public: Bullet *prev; Bullet *next; Bullet () {}; Bullet (/*POINT location,*/ double direction, int type); void doAI(); //Bullet returnNext(); //Bullet returnPrev(); }; Bullet::Bullet (/*POINT location,*/ double direction, int type) { cout << "new Bullet created" << direction << type; } void Bullet::doAI() { cout << "Doing AI..."; } /*Bullet Bullet::returnNext() { return *next; } Bullet Bullet::returnPrev() { return *prev; }*/ class List { private: Bullet *FirstBullet; Bullet *LastBullet; public: List(); List(Bullet *aBullet); ~List(); void doAI(); Bullet* AddBullet(/*LPoint location,*/ double direction, int type); void RemoveBullet(Bullet *aBullet); }; List::List() { FirstBullet = NULL; LastBullet = NULL; } List::List(Bullet *aBullet) { FirstBullet = aBullet; LastBullet = aBullet; } void List::doAI() { Bullet *temp_bullet; Bullet *temp_next; temp_bullet = FirstBullet; while(temp_bullet != NULL) { temp_next = temp_bullet->next; temp_bullet->doAI(); temp_bullet = temp_next; } } Bullet* List::AddBullet(/*POINT location,*/ double direction, int type) { Bullet *newBullet; newBullet = new Bullet(/*location,*/ direction, type); if (LastBullet == NULL) { FirstBullet =LastBullet = newBullet; } else { LastBullet->next = newBullet; newBullet->prev = LastBullet; LastBullet = newBullet; } return newBullet; } void List::RemoveBullet(Bullet *aBullet) { if (FirstBullet == aBullet) { Bullet *temp_next; temp_next = aBullet->next; temp_next->prev = NULL; FirstBullet = temp_next; delete aBullet; } else if (LastBullet == aBullet) { Bullet *temp_prev; temp_prev = aBullet->prev; temp_prev->next = NULL; delete aBullet; } else { Bullet *temp_prev; Bullet *temp_next; temp_prev = aBullet->prev; temp_next = aBullet->next; temp_prev->next = temp_next; temp_next->prev = temp_prev; delete aBullet; } } List::~List() { Bullet *temp_bullet; Bullet *temp_next; temp_bullet = FirstBullet; while(temp_bullet != NULL) { temp_next = temp_bullet->next; RemoveBullet(temp_bullet); temp_bullet = temp_next; } } void main () { Bullet *temp_bullet; /*LPoint tempPoint;*/ int temptype, tempX, tempY; double tempDouble; List myList; cout << "How many Bullets to create? "; int input; cin >> input; for (int i = 1; i<=input; i++) { cout << "\nInput x location for bullet#" << i; cin >> tempX; cout << "\nInput y location for bullet#" << i; cin >> tempY; /*tempPoint.x = tempX; tempPoint.y = tempY;*/ cout << "\nInput direction for bullet#" << i; cin >> tempDouble; cout << "\nInput type number for bullet#" << i; cin >> temptype; temp_bullet = myList.AddBullet(/*tempPoint, */tempDouble, temptype); } myList.doAI(); //return 0; } It performs an illegal operation (try compiling it). I have no idea what could be the problem because it seems right to me. E-mail: chaos1111@hotmail.com me (Mark)

Share this post


Link to post
Share on other sites
In your AddBullet, you do LastBullet->next = newBullet, as well as LastBullet = newBullet...Im not sure why you do this...

this would create a loop when you try to traverse the list and get to the LastBullet....

Share this post


Link to post
Share on other sites
The point of that code was:
1)LastBullet->next = newBullet, is supposed to mean that the Bullet object that LastBullet points to now has a link to newBullet.

2)LastBullet = newBullet, is supposed to make last Bullet now point to a different Bullet object. ie. newBullet.

Is this idea flawed somehow?

Share this post


Link to post
Share on other sites
I think your list insertion code is fine. The problem looks to me that you never initialize your Bullet.prev and Bullet.next pointers in your link list to NULL. You should do this in your Bullet constructor. Without it, you''ll get an infinite loop in doAI, I believe.

I didn''t actually test your code, so I may have overlooked something.

HTH

Share this post


Link to post
Share on other sites
It''s not just "good programming practice", it''s definitely required in your case, because you query those variables to terminate your links. Class member vars are not initialized to 0 as they are for statics.

Too bad that didn''t fix the problem, though! I notice you have two constructors: are you initializing the variables in both of them?

I would suggest booting up your debugger and just stepping through the procedure.

Goodl uck!

Share this post


Link to post
Share on other sites
Ok, I''ve been investigating with the debugger. I''ve discovered that the problem is with the RemoveBullet function, which is called by the List destructor. The exact line in the RemoveBullet function that it stops on is: temp_prev->next = temp_next;. This is in the final else statment.

Also, the doAI() function in the Bullet class is supposed to output text, but this never happens. This would suggest that the destructor is being called early. This really confuses me.I discovered that if I remove the List destructor entirely that the program works perfectly.

Now, I''m wondering, if you use dynamic memory, don''t you have to delete it? Why is it that when I try to delete it i get errors and when i don''t it works perfectly?

Share this post


Link to post
Share on other sites
I think your problem is here:

Bullet *temp_next;
temp_next = aBullet->next;
temp_next->prev = NULL;
FirstBullet = temp_next;
delete aBullet;


You are not checking to see if there is ONLY 1 bullet.

If you are deleting the first bullet, aBullet->next would be null.

therefore calling temp_next->prev = NULL; would cause an error, as being null, temp_next has no data to access.

Share this post


Link to post
Share on other sites
But wouldn''t my if statments take care of that problem:
if (FirstBullet = aBullet), else if (Last Bullet == aBullet)?

I think for some reason that these statments don''t work properly, so the final else statement is always being executed, but i don''t know what''s wrong

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
First, both constructors need to set prev and next to NULL.

Second, in RemoveBullet:
No way it could know if its the last. Change

Bullet *temp_next;
temp_next = aBullet->next;
temp_next->prev = NULL;
FirstBullet = temp_next;
delete aBullet;

To:

Bullet *temp_next;
temp_next = aBullet->next;
if (temp_next != NULL) temp_next->prev = NULL;
FirstBullet = temp_next;
delete aBullet;

Runs without a hitch now

Share this post


Link to post
Share on other sites