current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

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();
}
}
share|improve this question
up vote 1 down vote

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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