Proof read code for improvement

Hi, I know re-inventing the wheel is not something that's popular but I just wanted to create basic data structures in C++ and improve my coding. If any of the folks can go through the code and let me know of modifications, I'd appreciate it greatly.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
#ifndef __MYSTACK_H__
#define __MYSTACK_H__

template <class T>
class MyStack {
public:
    MyStack () : count(0) {
        topNode=new node;
    }
    
    virtual ~MyStack () {
    // code to delete nodes
        while (!empty())
            pop();
    }
    
    int size() const {
        return count;
    }
    
    bool empty() const {
        return count==0;
    }
    
    void push(T val) {
        node* tmp=new node;
        tmp->prev=topNode;
        topNode=tmp;
        topNode->value=val;
        ++count;
    }
    
    void pop() {
        node* tmp=topNode->prev;
        delete topNode;
        topNode=tmp;
        --count;
    }
    
    const T& top() {
        return topNode->value;
    }
private:
    /* data */
    int count;
    struct node {
        /* data */
        node() : prev(NULL) {}
        node *prev;
        T value;
    };
    node* topNode;
};

#endif /* __MYSTACK_H__ */ 
pop() has no protection from the stack being empty. If your intention is that pop() is only called from the destructor (which does have protection), then pop() should be private.

What's the value of topNode when the stack is empty? Since pop() is public, it's possible for the user to empty the stack.

What's the value of topNode->value after constructing MyStack?
IMO pop should return the value being popped as well and if there is nothing to pop then should throw some sort of exception so you can handle it accordingly.

Though it may be because I prefer the java/c# stack versus c++ stack.
http://msdn.microsoft.com/en-us/library/system.collections.stack%28v=vs.110%29.aspx
http://docs.oracle.com/javase/7/docs/api/java/util/Stack.html#push(E)

Where you have

peek -> returns top item
push -> pushes item to top and returns it
pop -> removes top item, returns the item that was removed
Doing what giblit saids with pop is probably the best method. A bit like how the standard library does with throwing underflow exception.
Made the changes. Glad that there weren't more mistakes. Thanks for your inputs.
Topic archived. No new replies allowed.