4
\$\begingroup\$

Could someone please review my code and let me know if there is any scope for optimization for better performance.

package com.gmail.practice;

import java.util.Arrays;

public class Stacks {

private int top;
int size;
int[] stack;

public Stacks(int arraysize)
{
    size = arraysize;
    stack = new int[size];
    top = -1;
}

public void push(int value)
{
    if(top == size-1)
    {
        System.out.println("the value cant be pushed: Stack underflow");
    }
    else{
        top = top+1;
        stack[top] = value;
    }
}

public void pop()
{
    if(top > size-1)
    {
        System.out.println("stack is overflow");
    }else{
        top = top-1;
        System.out.println("current top value is " + " " +stack[top]);
        System.out.println("the popped value is" + " " + stack[top+1]);
    }

}

public void display()
{
    for(int i=0;i<size-1;i++)
        System.out.println(stack[i]);
}

public static void main(String[] args)
{
    Stacks s = new Stacks(5);
    s.push(10);
    s.push(15);
    s.push(19);
    s.push(3);
    s.pop();
    s.push(4);
    s.display();
    s.pop();
    s.pop();
    s.display();
}
}
\$\endgroup\$
1
\$\begingroup\$

As mentioned in @rolfl's to your later question, there is limited value in this class as elements cannot be easily retrieved.

Putting aside the obvious improvements to the seemingly toy implementations, you have a mistake in describing whether the stack has 'overflow' or 'underflow': the meanings should be inversed. When you try to push(int) to a full stack, that is known as an overflow, and you need to check whether the stack is empty when pop()-ping instead of checking for 'overflows'.

To better indicate error conditions such as those mentioned above, you can make use of throwing Exceptions instead of a simple System.out.println(). In fact, I will suggest using System.err.println() at the very least to differentiate between normal and error 'outputs'.

A Java 8 way of printing the contents of the stack is to use Arrays.stream(int[]):

public void display() {
    Arrays.stream(stack).forEach(System.out::println);
}

This uses System.out.println(int) as a method reference to print each int value on the console.

Simply running through some test operations in the main() method is also barely enough, you should consider proper unit testing techniques such as using a testing framework that can arrange-act-assert for you.

Last but not least, you should also take a look at the standard JDK classes' stack implementations for greater inspiration, such as the Deque interface.

\$\endgroup\$
1
\$\begingroup\$

Why not just throw exceptions instead of writing something to the console? Also make use of generics? It is highly unlikely that Stack will be used only for integers. A stack implementation using arrays is good, but most of the time people also use LinkedList to implement it, so that you do not have to resize once you exceed INITIAL_CAPACITY. It all comes down to the user's choice.

Here are two implementations which can be useful:

Array implementation of Java Stack

My own Stack Implementation in Java

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.