• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
sooner123

Problem with destructors in C++

9 posts in this topic

Here is my code. I'm having two issues with destructors here.

I'm trying to write a destructor that will run through the linked list of children of the current node being destroyed and delete all of them (which should recursively call their destructors as well)

Instead the program just crashes.

The other issue is that this if I uncomment the code in RemoveAndDeleteAllWidgets, and try to manually call the destructor, I get an error saying 'no match for 'operator~' in '~Widget()'

This last error is making me suspect that my compiler might be buggy and have issues dealing with destructors.

[code]
~Widget()
{
cout << "children of node with value " << x << " being deleted" << endl;
Node*sentinel = new Node;
sentinel = list;
cout << sentinel->data->x << "*" << endl;
while (sentinel != NULL)
{
sentinel = sentinel->next;
delete sentinel->prev;
}
}
void RemoveAndDeleteAllWidgets()
{
//~Widget();
}
[/code] Edited by sooner123
0

Share this post


Link to post
Share on other sites
I don't want to sound harsh, but seriously, use std::list. Your code is leaking all over.
No, your compiler is surely not buggy. You don't call destructors directly.

Have you used the debugger and checked where it crashes?


The crash comes probably from that loop. Think about it, you're reaching the last element, so sentinel = sentinel->next; is setting sentinel to NULL.
And then you try to access sentinel->prev. Null pointer access!

[code]
sentinel = list;
cout << sentinel->data->x << "*" << endl;
while (sentinel != NULL)
{
sentinel = sentinel->next;
delete sentinel->prev;
}
[/code] Edited by Endurion
1

Share this post


Link to post
Share on other sites
The first crash is probably because you assign sentinel to sentinel->next and after that call delete sentinel->prev. This will work, except for the last one, when sentinel->next is NULL and you try to delete NULL->prev. You did see correct that you need to get next before deleting, because after calling delete you cannot access next anymore, but you should use a local variable here, like
[code]Node *next = sentinel->next;
delete sentinel;
sentinel = next;[/code]
And while it won't affect the crash, you call Node*sentinel = new Node, and then overwtite sentinel, leaking the memory. Probably not what you want.

About the second problem, calling a destructor yourself is evil, you should really try to avoid that. It's better to let your destructor call RemoveAndDeleteAllWidgets() instead if they share the same functionality. While there are ways to call a destructor explicitly, it's best to try working around it.
2

Share this post


Link to post
Share on other sites
Not harsh at all. I appreciate the response. And though I agree that my code is pretty much leaky garbage, I'd like to fine tune this and turn it into something that isn't garbage before I start using libraries to do this stuff for me.

And thank you for pointing out the error in the logic that was causing the crash.

One last mistake on my part. I was deleting the Node instead of the Class the Node pointed to.
1

Share this post


Link to post
Share on other sites
Note that calling new in a destructor is a bad idea, as is any operation that can cause an exception. Many of the C++ mechanisms for ensuring code correctness get thrown out the window if your destructors can throw, so at the minimum you need to trap those exceptions. But it's much better not to get into a situation where your operations can throw in the first place.
0

Share this post


Link to post
Share on other sites
Holy father of leakage.. what do we have here? [img]http://public.gamedev.net//public/style_emoticons/default/biggrin.png[/img]
My comments in bold, cause I am bald... no correction, they are in RED cause the code tag isn't bold enough to show bold.

[quote name='sooner123' timestamp='1354476891' post='5006350']
[code]
~Widget()
{
cout << "children of node with value " << x << " being deleted" << endl;
Node*sentinel = new Node; // WHOA?? new called in a destructor? for what?
sentinel = list; // AH! That's why called new! To leak it one line later. Good job!
cout << sentinel->data->x << "*" << endl;
while (sentinel != NULL)
{
sentinel = sentinel->next; // POTENTIALLY next is nullptr and if it is nullptr
delete sentinel->prev; // BAM, deferencing a null pointer... CRASH
}
}
void RemoveAndDeleteAllWidgets()
{
//~Widget();
}
[/code]
[/quote]

So, a total mess in one function, I can't imagine what else you have around in your code.
If you are learning that's all fine.. take your time and crash the entire world until you get it right.
If you are in production, do yourself a favour and #include <vector> . Edited by kunos
2

Share this post


Link to post
Share on other sites
My bet is that sooner123 doesn't understand the pointer concept very well.

sooner123,

[source lang="cpp"]Node* sentinel = new Node;[/source]

This line of code creates a new object of the [b]Node[/b] class on the free store and makes the sentinel pointer reference that object.

So in order to make your pointer reference the pre-existing [b]Node [/b]object, which you wanted to do in the first place, you just need this

[source lang="cpp"]Node* sentinel = list;[/source]

Sorry for stating the obvious to the vast majority of you, but from what I got from sooner123's responses, the op probably didn't understand why allocating the
new node was unnecessary. Edited by vdaras
1

Share this post


Link to post
Share on other sites
Note that "sentinel" is a poor variable name. When used with linked lists, the term "sentinel" means a special node value, which allows one to establish not-null invariants on the next or previous pointers.

A simpler and more accurate name might be "node" or "current".
1

Share this post


Link to post
Share on other sites
[quote name='vdaras' timestamp='1354545989' post='5006609']
My bet is that sooner123 doesn't understand the pointer concept very well.

sooner123,

[source lang="cpp"]Node* sentinel = new Node;[/source]

This line of code creates a new object of the [b]Node[/b] class on the free store and makes the sentinel pointer reference that object.

So in order to make your pointer reference the pre-existing [b]Node [/b]object, which you wanted to do in the first place, you just need this

[source lang="cpp"]Node* sentinel = list;[/source][/quote]I completely agree.
It very much demonstrates an incomplete knowledge of pointers and dynamic memory allocation, which being the case means that the destructor is bound to be just one of meny problems with the code, most of which has not been shown.

I would advise that you get yourself out of the mindset that if you fix this "last error" that you're even half way towards having a good linked-list class. That simply wont be the case at this point unfortunately. Learning is a process. You might have a good understanding of what it takes to make a good linked-list by the time you've written 5 different variations on them, leant all the ins and outs of [i]using [/i]an existing linked-list class or two, and learnt all about exception safety. I highly recommend the Guru of the week site: [url="http://www.gotw.ca/gotw/"]http://www.gotw.ca/gotw/[/url]
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0