patternjavaModerateCanonical
Array implementation of Java Stack
Viewed 0 times
arrayimplementationstackjava
Problem
As an exercise, I wanted to make an array implementation of
I'd like to know the following:
Note: I know Javadoc is missing, but I am only just beginning to read into that.
Stack, including most of the methods from the native Java Stack. I'd like to know the following:
- Does the code follow good programming principles?
- Is the code readable/understandable?
- Are there any redundant fields/operations?
- Is there a way to further improve the code?
- Should I reduce the capacity of the stack at some point? Testing the native Java Stack, it doesn't seem to do any size reduction, except with
trim().
Note: I know Javadoc is missing, but I am only just beginning to read into that.
import java.util.Arrays;
import java.util.EmptyStackException;
import java.util.Random;
@SuppressWarnings("unchecked")
public class ArrayStack {
private T[] data;
private int numElements;
private static final int INITIAL_CAPACITY = 10;
public ArrayStack() {
data = (T[]) new Object[INITIAL_CAPACITY];
}
private void enlarge() {
T[] temp = Arrays.copyOf(data, data.length);
data = (T[]) new Object[data.length * 2];
for (int i = 0; i < temp.length; i++) {
data[i] = temp[i];
}
}
public void push(T element) {
if (numElements == data.length) {
enlarge();
}
data[numElements] = element;
numElements++;
}
public T peek() {
if (isEmpty()) throw new EmptyStackException();
return data[numElements - 1];
}
public T pop() {
if (isEmpty()) throw new EmptyStackException();
T element = data[--numElements];
data[numElements] = null;
return element;
}
public boolean isEmpty() {
return (numElements == 0);
}
public int capacity() {
return data.length;
}
public int size() {
return numElements;
}
public void clear() {
for (int i = 0; i < numElements; i++) {
data[i] = null;
}
numElements = 0;
}
}Solution
Suppressing warnings
It's hard to defend suppressing warnings at the class level.
The problematic lines in your code are these:
Which appears in two places.
I suggest to move the array allocation to one place,
and suppress the warning only for that line, like this:
This hits two birds with one stone:
In case you were wondering, the local
Another important note here is the
The
The
Manual array copy
Instead of this:
It's better to use
(Also seen in the previous section.)
Other minor things
In
rather than simply creating a fresh new array.
You could let the garbage collector take care of the stale array.
It's hard to defend suppressing warnings at the class level.
The problematic lines in your code are these:
data = (T[]) new Object[INITIAL_CAPACITY];Which appears in two places.
I suggest to move the array allocation to one place,
and suppress the warning only for that line, like this:
public ArrayStack() {
data = newArray(INITIAL_CAPACITY);
}
private final T[] newArray(int capacity) {
@SuppressWarnings("unchecked")
T[] tmp = (T[]) new Object[capacity];
return tmp;
}
private void enlarge() {
T[] temp = Arrays.copyOf(data, data.length);
data = newArray(data.length * 2);
System.arraycopy(temp, 0, data, 0, temp.length);
}This hits two birds with one stone:
- The ugly array creation is now at once place instead of two
- Warnings are suppressed for the single problematic line, not for the entire class
In case you were wondering, the local
tmp variable is necessary in newArray, otherwise Java doesn't allow suppressing the warning on a return statement or regular assignment. It has to be declaration + assignment.Another important note here is the
private final modifier of newArray.The
private modifier is necessary to prevent sub-classes from overriding the behavior and potentially breaking the constructor.The
final modifier is necessary to prevent sub-classes from shadowing the method by another method with the same name, causing confusion.Manual array copy
Instead of this:
for (int i = 0; i < temp.length; i++) {
data[i] = temp[i];
}It's better to use
System.arraycopy instead:System.arraycopy(temp, 0, data, 0, temp.length);(Also seen in the previous section.)
Other minor things
In
clear, I'm wondering why you manually null out the elements,rather than simply creating a fresh new array.
You could let the garbage collector take care of the stale array.
Random is unused, so remove the import.Code Snippets
data = (T[]) new Object[INITIAL_CAPACITY];public ArrayStack() {
data = newArray(INITIAL_CAPACITY);
}
private final T[] newArray(int capacity) {
@SuppressWarnings("unchecked")
T[] tmp = (T[]) new Object[capacity];
return tmp;
}
private void enlarge() {
T[] temp = Arrays.copyOf(data, data.length);
data = newArray(data.length * 2);
System.arraycopy(temp, 0, data, 0, temp.length);
}for (int i = 0; i < temp.length; i++) {
data[i] = temp[i];
}System.arraycopy(temp, 0, data, 0, temp.length);Context
StackExchange Code Review Q#111944, answer score: 11
Revisions (0)
No revisions yet.