Proof read code for improvement

Dec 28, 2014 at 10:55pm
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__ */ 
Dec 28, 2014 at 11:39pm
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?
Dec 29, 2014 at 12:32am
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
Dec 29, 2014 at 12:39am
Doing what giblit saids with pop is probably the best method. A bit like how the standard library does with throwing underflow exception.
Dec 31, 2014 at 2:37am
Made the changes. Glad that there weren't more mistakes. Thanks for your inputs.
Topic archived. No new replies allowed.