Simple implementation of a generic stack I'm fairly new to C++ programming. To help I've been writing my own data structures for practice. I'd liked this to be judged as if it was written by a professional, and receive honest feedback. I already have a few concerns.


Am I managing memory correctly? I don't believe I have any leaks. A lot of advice I get when using pointers is to use smart pointers, but raw pointers are fine when the class encapsulates all pointers correct?
I'm unhappy my &lt;&lt; operator overload has to call to peekAll inside the class. I feel like I'm going about this in the wrong way. Aside from that should I overload any other operators? I didn't feel arithmetic operators made much sense.




#include &lt;ostream&gt;

template &lt;typename T&gt;
class BBStack {
public:
BBStack(T type) {
    head = new Node(type);
}

virtual ~ BBStack() {
    Node* temp = head;
    while (head != nullptr) {
        temp = head-&gt;next;
        delete head;
        head = temp;
    }
    delete temp;
}

void push(T type) {
    Node *newNode = new Node(type, head);
    head = newNode;
}

T peek() const {
    return head-&gt;data;
}

T pop() {
    if (head == nullptr) {
        std::cout&lt;&lt;"Error stack is empty"&lt;&lt;std::endl;
        return NULL;
    } else {
        Node *temp = head;
        T result = temp-&gt;data;
        head = head-&gt;next;
        delete temp;
        return result;
    }
}

std::ostream&amp; peekAll(std::ostream&amp; out) const {
    if (head == nullptr) return out &lt;&lt; "Stack is empty";
    Node* temp = head;

    while (temp != nullptr) {
        out &lt;&lt; temp-&gt;data &lt;&lt; " ";
        temp = temp-&gt;next;
    }

    delete temp;
    return out;
}

private:
    struct Node {
        Node(T type, Node* _next = nullptr) : data(type), next(_next) {}
        Node(Node* node) : data (node-&gt;data), next (node-&gt;next) {}

        T data;
        Node* next;
    };
    Node* head;

};

template &lt;typename T&gt;
std::ostream&amp; operator&lt;&lt; (std::ostream&amp; out, const BBStack&lt;T&gt;&amp; stack) {
    return stack.peekAll(out);
}
Ownership 

Yes raw pointers are fine if you are not transferring ownership (Is the class responsibility to delete them).

Pop function 

Also I am not sure if this will compile for any data other than pointers and types convertible to 0 since you return NULL. (Also is better to use nullptr consistently).

 T pop() {
    if (head == nullptr) {
        std::cout&lt;&lt;"Error stack is empty"&lt;&lt;std::endl;
        return NULL;
    } else {
        Node *temp = head;
        T result = temp-&gt;data;
        head = head-&gt;next;
        delete temp;
        return result;
    }
}


IMO it will be better to throw a underflow exception since is the client responsibility to check that is not empty before. Also avoid returning the data since we will be throwing an exception (For more info on this check out this article http://ptgmedia.pearsoncmg.com/images/020163371x/supplements/Exception_Handling_Article.html)

 void pop() {
    if (head == nullptr) 
        throw std::underflow_error("underflow");

    Node *temp = head;
    head = head-&gt;next;
    delete temp;   
 }


So the client will have to call peek before pop to get the same effect.

Parameters: 

In most parameter like this (T type) you should change it to (const T&amp; type)
to avoid copying. Even better would be to provide an overload for rvalue references like this (T&amp;&amp;)

peekAll function 

This function seams kind of weird. Why are you deleting temp? . It seams that after the while loop temp should always be nullptr.  And aren't you just writing the stack on the stream.  I would just omit this function and move it to the &lt;&lt; operator overload.

Also you should implement an empty function.