Exception error

Hi,
the program files that i have attached, when compiled runs fine except i get this huge error on the last exception that is caught...any ideas as to where i went wrong?

the output goes :
2
1
1
0
Stack if empty.
9
10
2.1
10
and as it tried to execute the last line i get this debug assertion failed error.

This is the correct output
2
1
1
0
Stack if empty.
9
10
2.1
10
Stack if full. Can hold only 10 elements

Your destructor is wrong. You aren't passing the same pointer to delete[] that you got from new[]. This is your assertation failure.

You also need a copy constructor and an assignment operator; the automatically-generated ones will not work.

I imagine the program would work fine with those changes.

thanks, but isnt stackPtr supposed to be deleted, bec it was allocated?

I was going to implement those 2(Law of big 3), thanks for that.

Quote:

Originally posted by noob2c
thanks, but isnt stackPtr supposed to be deleted, bec it was allocated?

I was going to implement those 2(Law of big 3), thanks for that.

Yes, it must be deleted. But it must be the same pointer you got from new.

Think: Where does stackPtr point when destruction happens? Does it point at its original location, or does it point somewhere else? Somewhere like "original location + counts" perhaps?

You must pass delete the same address you got from new. You can't pass it a different address (which is what you do, because stackPtr probably doesn't point at the first element of that array anymore).

aaah , i get you know, now just to figure out how to get it back to its original location
thanks

"now just to figure out how to get it back to its original location"

As Cat hinted, when you use delete, you are deleting the memory allocated--not the pointer. Afer you use delete on a pointer, the pointer still exists, and you can assign it a different address. (As an exercise, prove that to yourself by trying it.) How does that relate to your problem? You don't necessarily need to delete stackPtr. You could keep track of the address you allocated, with another pointer:

double * stackPtr = new double[10];

//declare a constant pointer(i.e. it cannot be assigned another address) to keep track of the address you allocated:

double* const location = stackPtr;
...
...

delete [] location;

7stud has given you the simplest answer. It does have the drawback of (possibly) requiring more memory.

There is, due to how you implement, though, an algebraic relationship that always holds (assuming your push and pop are correctly done, more on that later). This relationship is:

originalStackPtr + counts = stackPtr.

This means you truly only need to keep two of these three as variables -- the third quantity is redundant and can always be generated from the other two. I.e.:

originalStackPtr = stackPtr - counts;
counts = stackPtr - originalStackPtr;
stackPtr = originalStackPtr + counts;

so by storing any two of these quantities, you have effectively stored all three.

Technically, the best one to leave out is count -- both count and stackPtr change their values with a push and pop, so omitting one of those two makes less work in the push() and pop() functions. But you could omit storing any of these three quantities and recover it whenever you needed, given the other two.

Edit:

There is also a problem (admittedly very subtle) with this code:

Code:

template<class T> void Stack<T>::push(const T& a) { if(counts<sizeOfstack) { *stackPtr++=a; counts++; } else throw StackExceptions::StackFull(sizeOfstack); }

Can anyone correctly identify why the following code is preferable? That is, why/how could the first code fail but the second succeed?

Code:

template<class T> void Stack<T>::push(const T& a) { if(counts<sizeOfstack) { *stackPtr=a; stackPtr++; counts++; } else throw StackExceptions::StackFull(sizeOfstack); }

I think i got it

before deleting it I did this

stackPtr=0;
delete [] stackptr;

and now it works fine, is that the right thing to do by the way?

Quote:

Originally posted by noob2c
I think i got it

before deleting it I did this

stackPtr=0;
delete [] stackptr;

and now it works fine, is that the right thing to do by the way?

No; you're passing a null pointer to delete, which does nothing. You're leaking memory with every object.

The correct deletion would be "delete[] (stackPtr-counts);". If stackPtr has been incremented "counts" times, then the way to undo this is to subtract "counts".

Code:

---------------------------- dynamically allocated memory|| ---------------------------- ^ | | stackPtr NULL or 0 _______________ other memory / ______________/

stackPtr = &of_something_else;

Code:

---------------------------- dynamically allocated memory|| ---------------------------- stackPtr NULL or 0 | | \/ ________________ other memory / _______________/

stackPtr = 0;

Code:

---------------------------- dynamically allocated memory|| ---------------------------- stackPtr ---------------------> NULL or 0 ________________ other memory / _______________/

delete [] stackPtr;

That line doesn't delete the dynamically allocated memory. Remember when you delete a pointer, you are deleting the memory it points to *NOT* the pointer itself. So, it doesn't matter what pointer you delete as long as the pointer is pointing to the dynamically allocated memory. The goal is to delete the dynamically allocated memory not a certain pointer.

Cat, what is the subtle problem you posted? I don't get it.

Code:

*stackPtr++=a;

Wouldn't that increment *stackPtr before it assigns it to a?

Here is an explanation of the "subtle error". Consider again the original code:

Code:

template<class T> void Stack<T>::push(const T& a) { if(counts<sizeOfstack) { *stackPtr++=a; counts++; } else throw StackExceptions::StackFull(sizeOfstack); }

Well, this does three things, and in this order:

* Increments stackPtr.
* Assigns a to the location pointed at by the old version of stackptr.
* Increments counts.

The other code does things in this order:

* Assigns a to the location pointed at by the old version of stackptr.
* Increments stackPtr.
* Increments counts.

Now, could any of these fail? Incrementing a pointer cannot fail, nor can incrementing an int. But, alas! -- the assignment operator. It can't fail *for primitive types*, but we're templated on this type, so it could be a class. And, if it is, it could throw.

In the first case, if assignment throws, stackPtr WILL still get incremented, the object we tried to assign to (which is now "on the stack") is invalid, and the count is wrong. The destructor will delete the wrong pointer and all the program ends in fiery doom.

In the second case, if the assignment throws, stackPtr will NOT get incremented, nor will count. We have a failed assignment, but the new element wasn't yet officially on the stack anyway (stackPtr wasn't incremented). We trust that whoever wrote the contained class made sure that failed assingments don't leave the object in an impossible or unusable state; that's nothing we can do anything about, the burden is on the author of the contained class. Ideally a failed assignment leaves the object in its pre-assignment state.

So it essentially did nothing: No increments, and the failed assignment didn't screw anything up (as long as the contained class is well-written).

Essentially, we want this function to do three things: assign and two increments. If we code this correctly, we should have an "all or nothing" success. If it can't accomplish all three, it should accomplish none of them. The first code does not; it could accomplish one of them but not the other two. The second code is "commit or rollback" because the push either succeeds completely or fails completely.

Wow, I never would have noticed.

Speaking of assignment, wouldn't it be better to use the approach the STL uses and initialize the new element using placement new instead of the assignment operator?
Would require a non-initializing memory allocation of course - but that might be a good thing anyway.