Implementing a stack with Java

Question

I am trying to implement a stack with Java:

public class Stack<E> {
 Node top = null;
 public void push(E data) {
 Node node = new Node(data);
 if (null != top) {
 node.next = top;
 }
 top = node;
 }
 public E pop() {
 if (null == top) {
 throw new IllegalStateException("Stack is empty");
 } else {
 E data = top.data;
 top = top.next;
 return data;
 }
 }
 public E peek() {
 if (null == top) {
 throw new IllegalStateException("Stack is empty");
 } else {
 E data = top.data;
 return data;
 }
 }
 public void print() {
 if (null == top) {
 throw new IllegalStateException("Stack is empty");
 } else {
 Node node = top;
 while(null != node) {
 System.out.println(node.data);
 node = node.next;
 }
 }
 }
 private class Node {
 Node next;
 E data;
 Node(E data) {
 this.data = data;
 }
 }
}

I am looking for feedback for the implementation. Also, I wanted to know if the pop() implementation can cause a memory leak?

Answer 1

I wanted to know if the pop() implementation can cause a memory leak?

It will not cause a memory leak, because java is a garbage collected language. When top is no longer accessible the garbale collecter will free top if it sees fit.

feedback:

1. In push() the null check is unnessecary, because node.next is already null by default.

   public void push(E data) {
    Node node = new Node(data);
    node.next = top;
    top = node;
   }
   

2. Rather then have a print() method, create a toString() method. toString() is a method of Object that is used across java to get a string representation of an object. This will make it easier to print your object to different streams.

3. In Node, create a getData() (and optionally setData()) method to control access to the data class, or make data final.

4. make top private to control use of your class to the supported methods.

Answer 2

Printing an empty stack should not cause an exception. It should print nothing (or possibly print something that represents an empty stack).

There is no way to tell if the stack contains anything, other than peeking and catching an exception. Add a size() method, or at the very least isEmpty().