HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Java Max-Stack implementation to return maximum value in the stack

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
themaximumstackreturnjavavaluemaximplementation

Problem

I have written the below program to write a Java stack implementation. I have added a method which returns the maximum value in the stack, e.g, a pop. The implementation for this is based on the Max Heap Sort algorithim. I have little doubt on the time complexity analysis - it's not \$O(\log n)\$ is it?

public class Stack {

    int top = -1;
    int[] stack;

    public Stack(int size) {
        stack = new int[size];
    }

    public void push(int v) {
        if (top == stack.length - 1) {
            System.out.println("Oveflow!!");
            return;
        }
        stack[++top] = v;
    }

    public void pop() {
        if (top = 0; j--) {
            buildMaxHeap(c, j, c.length - 1);
        }

        return c[0];
    }

    private void buildMaxHeap(int[] clone, int i, int j) {
        int root = i;

        while (root * 2 + 1 = 0; i--) {
            System.out.println(stack[i]);
        }
    }
}

Solution

Running time:

The running time is not O(lg(n)) is O(n) since you are building a max heap.

You iterate over elements (no leaf elements). Knowing this we can conclude that is at least O(n).
The function called buildMaxHeap is actually O(h), h being the height of the node (which you call i). So because the height of any heap is



and because this summation to the infinity ends up being constant:

we get O(n).

Names:

buildMaxHeap doesn't actually builds the max heap so it name is not appropriate. This function is commonly called maxheapify and is to maintain the heap property. Also I would change the name of parameter named clone for something more generic, because in that context it doesn't matter if its a clone or not.

Pop:

Pop doesn't return the element poped, neither I see a top function so this stack is pointless.

Code

This part:

if ((c + 1) <= j && clone[c] <= clone[c + 1]) {
     c = c + 1;
 }


Can be change to this

if ((c + 1) <= j && clone[c] < clone[c + 1]) {
     ++c;
 }


Because if both are equal why bother incrementing c ?

This is not that important though I am just nitpicking.

This part could be improved as well:

if (clone[root] < clone[c]) {
    int t = clone[root];
    clone[root] = clone[c];
    clone[c] = t;
    root = c;
 } else {
    root++;
 }


For the swapping it would be more readable to implement a method named swap. Also if there is no swapping the algorithm should terminate, because the lower subtrees are already max heaps so its pointless. This is the reason why building a max heap starts at floor(n/2).

so this part would end up like this

if (clone[root] >= clone[c]) break; 

 swap(clone, root, c);
 root = c;


Conclusion:

Given the hidden constant factor in your function getMax (cloning the array and the hidden constant factor in building a max heap) its better to do it the straightforward way:

int max = Integer.MIN_VALUE;
    for(int i = 0; i <= top; ++i)
        max = Math.max(max, stack[i]);


or for Java 8:

OptionalInt i = Arrays.stream(stack,0, top+1).max();


Additional notes:
This also can be improved with Java generics.

Code Snippets

if ((c + 1) <= j && clone[c] <= clone[c + 1]) {
     c = c + 1;
 }
if ((c + 1) <= j && clone[c] < clone[c + 1]) {
     ++c;
 }
if (clone[root] < clone[c]) {
    int t = clone[root];
    clone[root] = clone[c];
    clone[c] = t;
    root = c;
 } else {
    root++;
 }
if (clone[root] >= clone[c]) break; 

 swap(clone, root, c);
 root = c;
int max = Integer.MIN_VALUE;
    for(int i = 0; i <= top; ++i)
        max = Math.max(max, stack[i]);

Context

StackExchange Code Review Q#105424, answer score: 2

Revisions (0)

No revisions yet.