#include<iostream>
Avoid <iostream> in headers unless you absolutely need it. Static constructors are transparently injected into every translation unit that includes <iostream>, which also means every translation unit that includes your header. If you want to provide IO utilities, separate those IO functions from your data structure.
Where is the header guard to prevent guard against multiple inclusion?
template<typename T>
struct Node {
T data;
Node *next;
Node(T data, Node *next) : data(data), next(next) {}
};
Hide implementation details from the users of your library.
Is aggregate initialization not good enough? Do you need a default constructor?
Stack() : top(nullptr), level(0), max_size(10) {}
Stack(int max_size) : top(nullptr), level(0), max_size(max_size) {};
...
private:
Node<T> *top;
int level, max_size;
When initializing data members with constant initializers, prefer in-class member initializers.
Stack() = default;
Stack(int max_size) noexcept : max_size(max_size) {};
...
private:
Node<T>* top{nullptr};
int level{0};
int max_size{10};
void push(T data);
For arguments are cheap to copy, prefer to pass by value. When arguments are expensive or unknown, prefer to pass by reference to const. There are exceptions, like arguments that are consumed, but in general, the two above rules cover most cases.
bool isEmpty() { return level == 0; }
bool isFull() { return level == max_size; }
int geLevel() { return level; }
int getMaxSize() { return max_size; }
None of these functions modify the stack and should be accessible in a const context. You should specify these member functions with const.
Prefer following standard naming conventions so your stack can be used with existing library solutions. isEmpty() should be empty(), geLevel() should be size(). getMaxSize() should be max_size(), or even capacity().
The built-in types are convertible to bool. For int types, any non-zero value is true.
bool empty() const { return !full(); }
bool full() const { return level; }
int size() const { return level; }
int max_size() const { return max_size; }
private:
Node<T> *top;
int level, max_size;
Only one variable declaration per line.
template<typename T>
void Stack<T>::push(T data) {
if(!isFull()) {
Node<T> *newNode = new Node<T>(data, top);
top = newNode;
level++;
} else {
std::cerr << "stack overflow";
}
}
If you are checking for overflow, you should do something to notify the caller that the operation failed besides logging to the console.
template<typename T>
T Stack<T>::pop() {
if(!isEmpty()) {
Node<T> *temp = top;
T data = temp->data;
top = top->next;
level--;
delete temp;
return data;
}
std::cerr << "stack underflow";
}
What is the responsibility of pop()? The function tells me its removing the top element. What does making a copy of the data and returning it have to do with removing the top element? If someone wants the top element, then there should use the accessor for it (peek()?).
Focusing on design, what exactly makes this stack an abstract data type? Where is the abstraction? I would expect a stack that defined certain operations over any type of container that maintained insertion order and allowed access, insertion, and deletion from the same end. A queue is similar, except insertion is done on the opposite end from access and deletion. Read up on the adaptor pattern.
Is stack an appropriate name? Because it doesn't support growth beyond a fixed size, perhaps you should call it a Fixed Stack?
Because your Stack is managing a responsibility, a resource (specifically the lifetime management of dynamically allocated nodes), you should consider how the five special member functions behave in that context.
~Stack() = default; // Doesn't delete allocated nodes
Stack(Stack const&) = default; // Doesn't deep copy
Stack(Stack &&) = default; // Does deep move
Stack& operator=(Stack const&) = default; // Doesn't deep copy
Stack& operator=(Stack &&) = default; // Does deep move
Since C++11, move operations became standardized. Consider supporting them (move construction, move assignment, move-based swap/push/emplace).
If you would like your containers to work with standard facilities, consider looking at the requirements outlined by the standard.
http://eel.is/c++draft/container.requirements.general
http://eel.is/c++draft/stack
pop()in the standard version returnsvoidis that a pop that returns a value can not be made exception safe. So instead they use two methodstop()this retrieves a reference to the top value andpop()simply removes the top value. \$\endgroup\$T. \$\endgroup\$