Stack implementation with StackInterface

Question

The code that I wrote is as follows:

public class Astack implements StackInterface {
private int sz;
private Object[] a;
private int topPosition = -1;
public Astack(int sz){
 a = new Object[sz];
 this.sz = sz;
}
public int size(){
return topPosition+1;
}
public boolean isEmpty(){
 if(topPosition == -1){
 return true;
}
 return false;
}
public void push(Object element) throws StackFullException{
 if(topPosition+1 == sz){
 throw new StackFullException("Stack Full.");
 }
 a[++topPosition] = element;
}
public Object top() throws StackEmptyException{
 if(topPosition == -1){
 throw new StackEmptyException("Stack already Empty");
 }
return a[topPosition];
}
public Object pop() throws StackEmptyException{
 if(topPosition == -1){
 throw new StackEmptyException("Stack already Empty");
 }
 return a[topPosition--];
}
}

The StackInterface that I wrote:

public interface StackInterface{
 public int size();
 public boolean isEmpty();
 public Object top() throws StackEmptyException;
 public void push(Object element) throws StackFullException;
 public Object pop() throws StackEmptyException;
}

• 200_success: Is this a homework problem? Is StackInterface given to you, or did you write it too? Could you also include it in the question?
• Abhay_maniyar: @200_success.I - is not a homework problem. I also wrote StackInterface. I just wanted to use stack the traditional way than using Stack java class. I have added the interface that i wrote in the question above.

Answer 1

private int sz;

Rather than call this sz, consider calling it capacity. Writing out whatever name is easier to read, and capacity makes it clearer that it is potential size not current size. But you don't actually need to store it. It's already stored as a.length.

public Astack(int capacity){
 a = new Object[capacity];
}

See, more readable.

public int size(){
return topPosition+1;
}

It's more customary to track the number of items in the stack, not the top position.

public int size() {
 return size;
}

Put those together.

public class Astack implements StackInterface {
 private Object[] a;
 private int count = 0;
 public Astack(int capacity){
 a = new Object[capacity];
 }
 public int size(){
 return count;
 }
 public boolean isEmpty() {
 return count == 0;
 }
 public void push(Object element) throws StackFullException {
 if (count == a.length) {
 throw new StackFullException("Stack Full.");
 }
 a[count] = element;
 count++;
 }
 public Object top() throws StackEmptyException {
 if (isEmpty()) {
 throw new StackEmptyException("Stack already Empty");
 }
 return a[count - 1];
 }
 public Object pop() throws StackEmptyException {
 if (isEmpty()) {
 throw new StackEmptyException("Stack already Empty");
 }
 --count;
 return a[count];
 }
}

Now each line only does one thing. We don't update count and return something else on the same line.

We don't have to do extra math to turn topPosition into what we really want.

 private Object[] a;
 private int count = 0;
 public Astack(int capacity){
 a = new Object[capacity];
 }

In the future, you may want to replace these lines with just

 private List<T> stack = new ArrayList<>();

Then the default constructor works, so you don't need to define one. And we don't need to maintain a count, as stack.size() will return the current count.

We can get rid of the StackFullException, as the ArrayList will automatically resize as necessary.

And we can stop casting to and from Object, as generics don't require that.

• Clashsoft: I think top should return a[count - 1].

Answer 2

There isn't really much to optimize here, but there are definitely improvements you can make regarding usability and coding style.

• use generics instead of Object, otherwise the stack is difficult to use in practice.
• in practice, you would probably want a stack to increase its size automatically instead of throwing an exception.
• your indentation is not consistent, making your code difficult to read.
• you need more vertical whitespace, for example before a new method
• your variable names could use improvement. What's a sz? What's an a? What's an Astack?

• Abhay_maniyar: I do intend my code properly but when i paste it here it just gets distorted which is very irritating. Apart from that you are right and +1 for the tips :)
• Clashsoft: Also, the use of checked exceptions makes this very unusable at the call site because you need try catch everywhere.
• tim: @Clashsoft definitely. But I wasn't sure if OP maybe extended eg RuntimeException, making the exceptions unchecked (which would seem like a good option here).
• Abhay_maniyar: @Clashsoft - That's correct. I am using couple of try catch blocks here.

Answer 3

You don't need an if statment just this:

public boolean isEmpty(){
 //Returns a boolean value
 return topPosition==-1;
}

When comparing 2 variables you get a booleanresult