delete[] doesnt work

This is a discussion on delete[] doesnt work within the C++ Programming forums, part of the General Programming Boards category; i am tryinh to create stack using linked list. function below is to pop the last element and my problem ...

  1. #1
    Registered User
    Join Date
    Aug 2009
    Posts
    30

    Arrow delete[] doesnt work

    i am tryinh to create stack using linked list.
    function below is to pop the last element and my problem is that the previous ptr is the same as current ptr though i did delete it after returning the string to the function.

    where did i do wrong?


    Code:
    std::string Stack::pop()
    {  
    	
    	node *ptr, *ptr2 = NULL;
    
                    //empty
    	if (start == NULL ){
    		return "";
    	}
    
    	//more items
    	for (ptr = start ; ptr->next != NULL ;ptr=ptr->next){
    		ptr2=ptr;
    		cout << "ptr2: " << ptr2->data << endl;
    		cout << "ptr: " << ptr->data << endl;
    	}
      
    	std::string ans = ptr->data;
    		
    
    	return ans;
    	delete ptr;
    
    }

    thanks.

  2. #2
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by effa View Post
    i am tryinh to create stack using linked list.
    function below is to pop the last element and my problem is that the previous ptr is the same as current ptr though i did delete it after returning the string to the function.

    where did i do wrong?


    Code:
    std::string Stack::pop()
    {  
    	
    	node *ptr, *ptr2 = NULL;
    
                    //empty
    	if (start == NULL ){
    		return "";
    	}
    
    	//more items
    	for (ptr = start ; ptr->next != NULL ;ptr=ptr->next){
    		ptr2=ptr;
    		cout << "ptr2: " << ptr2->data << endl;
    		cout << "ptr: " << ptr->data << endl;
    	}
      
    	std::string ans = ptr->data;
    		
    
    	return ans;
    	delete ptr;
    
    }

    thanks.
    Maybe it's because you didn't listen to the warning that the compiler should have been screaming at you about an unreachable code path. How can it get to the delete ptr; statement if you have a return right before it?
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  3. #3
    Registered User
    Join Date
    Aug 2009
    Posts
    30
    but i tried to return after the delete.it didnt work because of memory leak.to think of that,it does makes because if i did that means im trying to something that doesnt exist..correct me if im wrong...

    plus,i didnt receive any errors or warnings....

  4. #4
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    If you're using Visual C++, change to Warning Level 4 in the project settings. If you're using gcc, use the -Wall compiler flag.

    I think I'll need to see more code to figure out what's wrong. Either that or I need more sleep.
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  5. #5
    Registered User
    Join Date
    Aug 2009
    Posts
    30
    hope anyone here can point out where my mistake was.tyvm.

    h. file

    Code:
    class node {
    
    	public:
    			  std::string data;
    			  node *next;
    };
    			
    
    class Stack
    {
        public:
    		 node *start, *last;
    		 unsigned long int i;
    
             Stack();
    		 void push(std::string n);
    		 std::string pop();
             unsigned long count();
             ~Stack();
    
    };

    cpp file

    Code:
    #include <string>
    #include <iostream>
    #include "stack.h"
    
    using namespace std;
    
    Stack::Stack()
    {
    	
         start =NULL;
    	 last=NULL;
    	 i =0 ;
    	
    	
    }
    
    void Stack::push(std::string n)
    {
    	node*ptr;
    	ptr=new node;
    	ptr->data = n;
    	ptr->next = NULL;
    
    	if(start == NULL){
    		
    		start = ptr;
    		last = ptr;	
    		
    		}
    	else{
    		last->next = ptr;
    		last = ptr;
    			
    	}
    
    }
    
    std::string Stack::pop()
    {  
    	
    	node *ptr, *ptr2 = NULL;
        //empty
    	if (start == NULL ){
    		return "";
    	}
    
    	else{
    	//more items
    		for (ptr = start ; ptr->next != NULL ;ptr=ptr->next){
    			ptr2=ptr;
    			cout << "ptr2: " << ptr2->data << endl;
    			cout << "ptr: " << ptr->data << endl;
    		}
      
    		std::string ans = ptr->data;
    		
    		cout << "ptr2: " << ptr->data << endl;
    		
    
    		return ans;
    		
    	}
    	delete ptr;
    
    }
    
    unsigned long Stack::count()
    {
       node * ptr;
       
      for( ptr = start ; ptr != last ; ptr = ptr->next)
      {
    	   i++;
      }
    	
       cout << i;
       return i;
    }
    
    
    Stack::~Stack()
    {
       node *ptr;
    
      while( start != NULL )
       {
          ptr = start->next;
          delete start;
          start = ptr;
       }
    
    }
    
    //
    //int main(){
    //	Stack s;
    //	s.push("HELLO");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.pop();
    //	s.pop();
    //	s.pop();
    //}
    Last edited by effa; 09-17-2009 at 10:14 PM.

  6. #6
    Registered User
    Join Date
    Sep 2009
    Posts
    2

    Slight Rewrite

    I think the pop works better when the value is right there, instead of on the other end of the list. In other words, O(1), rather than an O(n) search. Since it's a stack, you know the value's always immediately available:

    Code:
    #ifndef STACK_H
    #define STACK_H
    
    class node {
    
    public:
        std::string data;
        node *next;
    };
    			
    class Stack
    {
    public:
        node *last;
        unsigned long int i;
    
        Stack();
        void push(std::string n);
        std::string pop();
        unsigned long count();
        ~Stack();
    };
    
    #endif // STACK_H
    Code:
    #include "stdafx.h"
    
    #include "Stack.h"
    
    using namespace std;
    
    Stack::Stack()
    {
      last=NULL;
      i =0 ;
    }
    
    void Stack::push(std::string n)
    {
      node*ptr;
      ptr=new node;
      ptr->data = n;
      ptr->next = NULL;
    
      if( last == NULL )
      {
        /* It's empty */
        last = ptr;
      }
      else
      {
        /* Not empty */
    
        /* Store the current last. */
        node *prevLast = last;
    
        /* This is the new last */
        last = ptr;
    
        /* The next-to-last */
        last->next = prevLast;
      }
    }
    
    std::string Stack::pop()
    {  
      node *ptr, *ptr2 = NULL;
    
      if( last == NULL )
      {
        return "";
      }
    
      std::string ans = last->data;
    
      ptr = last->next;
      delete last;
      last = ptr;
    
      return ans;
    }
    
    unsigned long Stack::count()
    {
      /*
       node * ptr;
       
      for( ptr = start ; ptr != last ; ptr = ptr->next)
      {
        i++;
      }
    	
       cout << i;
       return i;
       */
      return 0;
    }
    
    
    Stack::~Stack()
    {
       node *ptr;
    
      while( last != NULL )
       {
         cout << "Cleaning up " << last->data << endl;
          ptr = last->next;
          delete last;
          last = ptr;
       }
    }
    
    //
    //int main(){
    //	Stack s;
    //	s.push("HELLO");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.push("WORLD");
    //	s.pop();
    //	s.pop();
    //	s.pop();
    //}

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,305
    Quote Originally Posted by effa View Post
    hope anyone here can point out where my mistake was.tyvm.
    Your main mistake is that you didn't tell us what was wrong with the code you had. You should also politely ask a question rather than just tossing the code on the floor and hoping someone will take care of everything. We are not your mother and do not do your laundry for you!

    Code:
    std::string Stack::pop()
    {  
    	
    	node *ptr, *ptr2 = NULL;
        //empty
    	if (start == NULL ){
    		return "";
    	}
    
    	else{
    	//more items
    		for (ptr = start ; ptr->next != NULL ;ptr=ptr->next){
    			ptr2=ptr;
    			cout << "ptr2: " << ptr2->data << endl;
    			cout << "ptr: " << ptr->data << endl;
    		}
      
    		std::string ans = ptr->data;
    		
    		cout << "ptr2: " << ptr->data << endl;
    		
    
    		return ans;
    		
    	}
    	delete ptr;
    
    }
    You don't understand what's going on there. You're still trying to do the delete after the return. Both the if and the else branch explicitly return and so never reach the end of the function.
    You don't appear to have turned your warning level up, as was so rightfully suggested. Your compiler will tell you about mistakes like these if you let it. If neither of the options suggested apply to the compiler you are using, please tell us what compiler it is so that we may tell you how to allow your particular compiler to warn you.

    Saying something like "delete doesn't work" is like saying "my sword doesn't work", for which the problem is that you're clearly holding it at the wrong end. "The pointy end goes in the other guy"
    Which incidentally is also the problem with your stack operations. Your Push and Pop functions must do their thing at the head end of the list.

    goatless: As tempting as it is to do someone's homework for them when it's so easy for you, you're doing everyone a disservice by doing so.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  8. #8
    The larch
    Join Date
    May 2006
    Posts
    3,573
    You also never adjust the value of last.

    As mentioned earlier, to implement a stack it is much easier to push and pop at the beginning of the singly linked list. What matters is that items that are added last come out first. As long as this holds, it doesn't matter whether the items are added to the end or to the beginning of the list.
    Last edited by anon; 09-18-2009 at 03:58 AM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  9. #9
    Registered User
    Join Date
    Sep 2009
    Posts
    2
    Quote Originally Posted by iMalc View Post

    goatless: As tempting as it is to do someone's homework for them when it's so easy for you, you're doing everyone a disservice by doing so.
    I'll have to plead no contest to that charge. I should have left it at reconsider getting the data from the beginning of the end.

    I thought he stated the problem, though. He said last is the same as its previous pointer. My rough guess was that he wasn't cleaning up the last item correctly.

    And also, the homework really isn't complete, though the core functionality is there. The class needs to handle the assignment operator and to implement counting.

  10. #10
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,625
    I think I must repeat the age old tale:
    Just because you get an error after you make something right, doesn't necessarily mean that it's wrong.
    Moving the delete before the return is right. But it doesn't solve all problems. It merely solves one problem, but it is nevertheless right.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  11. #11
    ...and never returned. StainedBlue's Avatar
    Join Date
    Aug 2009
    Posts
    168
    1)Count should not be that complicated. If you push, increment a count, if you pop, decrement it.

    2)If you listened to me in your last thread, I warned about trying to return a value from pop(). Are you beginning to understand why?...

    HINT: Implement a top() function. Make pop() return void (i.e. nothing)

    3)Again, 2 pointers are not needed to implement a stack. Think about these two cases:

    a) you have one item in the stack.

    b) you have 0 items in the stack.

    How can you test for these conditions?

    Well....
    Code:
    // if this is roughly the node
    
    struct Node{
    
      std::string str;
      Node* next;
    };
    
    // and this is your stack
    
    class Stack{
    
      // ... stuff ...
    
      private:
      
        Node* head;
    };
    
    // Then you can do this:
    
    if( head->next == 0 ) // We have one item in the stack
    
    // and
    
    if( head == 0 ) // We have 0 items in the stack
    If you're having difficulty conceptualizing a "stack", think literally of a stack of plates. You put plates on the top, and you remove plates from the top. You wouldn't dare try and remove a plate from the bottom.

    This means, we only care about the top item. While yes, it's necessary to "link" them all, there's no need to have a pointer to the "last" or "bottom" element, we simply don't care.

    Now, special care needs to be taken when "popping" items off the stack (we need to know if there's more than one, one, or zero items) but again, if you move the pointers around correctly, you can use the above code to check for those conditions without necessitating the use of two Node pointers in your stack class.
    Last edited by StainedBlue; 09-18-2009 at 01:28 PM.

  12. #12
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,305
    Quote Originally Posted by goatless View Post
    And also, the homework really isn't complete, though the core functionality is there. The class needs to handle the assignment operator and to implement counting.
    That's very much true. You gave some working correct code for some of it, but there was plenty left to do. In fact leaving the head called 'last' was a nice confusing touch for anyone who might take the code verbatim

    Job of raising awareness about homework done.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. delete[] problem with release config...
    By mikahell in forum C++ Programming
    Replies: 8
    Last Post: 08-21-2006, 10:37 AM
  2. getline() don't want to work anymore...
    By mikahell in forum C++ Programming
    Replies: 7
    Last Post: 07-31-2006, 10:50 AM
  3. Why don't the tutorials on this site work on my computer?
    By jsrig88 in forum C++ Programming
    Replies: 3
    Last Post: 05-15-2006, 10:39 PM
  4. fopen();
    By GanglyLamb in forum C Programming
    Replies: 8
    Last Post: 11-03-2002, 11:39 AM
  5. DLL __cdecl doesnt seem to work?
    By Xei in forum C++ Programming
    Replies: 6
    Last Post: 08-21-2002, 04:36 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21