In Java, the Stack
class is not a good one to use. It has been superseded by the Deque
interface. The Stack class documentation has this to say: "A more complete and consistent set of LIFO stack operations is provided by the Deque interface and its implementations, which should be used in preference to this class."
Multiple different concrete classes implement the Deque
interface, and I would choose the ArrayDeque
class. You can still call the variable stack
.
Note that for challenges like this, it's common to skip things like input validation, and edge cases.... but you need to know that you are doing this. I would recommend leaving comments in places where you would normally have checks. Checks that you are missing are things like:
- more pops than pushes (resulting in exceptions when popping too far)
- values that are not valid in input (incorrect queries, non-valid integers in string format).
- "max" on empty stacks
Further, the actual use-case in this challenge only requires that you track the maximum value at each level in the stack. You don't need to track the actual value at all. To make this easier, you can "seed" the stack with a guaranteed minimum value Integer.MIN_VALUE
.
Your class has been set up using a bunch of static fields. This is not good practice. Your Stacker
class should have regular fields, and your main
method should create an instance of the class which has its own methods (not static methods).
The worst problem here is the output
field which is a horrible way to use a static
variable. In Java 8 you should consider passing in a callback function to handle output, or returning an optional in a stream. The optional is a better way....
All told, I would restructure your code to be similar to:
public class Stacker {
private final Deque<Integer> stack = new ArrayDeque<>();
public Optional<Integer> processOperation(String op) {
char opc = op.charAt(0);
switch(opc) {
case '1':
int previous = stack.isEmpty() ? Integer.MIN_VALUE : stack.peek();
int max = Math.max(previous, Integer.parseInt(op.substring(1).trim()));
stack.push(max);
return Optional.empty();
case '2':
if (stack.isEmpty()) {
throw new IllegalStateException("Cannot pop empty stack");
}
stack.pop();
return Optional.empty();
case '3':
if (stack.isEmpty()) {
throw new IllegalStateException("Cannot max empty stack");
}
return Optional.of(stack.peek());
default:
throw new IllegalStateException("Unknown operation:" + op);
}
}
}
That processOperation
returns max values when the operation is a 3
op. The Optional is empty for push/pop operations.
Simpler: You can then feed instructions in to it in a loop, and collect the results of the max when the Optional has a value....
... or ....
Advanced: You can then use this in a stream of commands:
Stacker stacker = new Stacker();
String report = commands.map(stacker::processOperation)
.filter(Optional::isPresent)
.map(opt -> String.valueOf(opt.get()))
.collect(Collectors.joining("\n"));
The above takes a stream of commands (commands
) and runs each command through the stacker, keeping only the max results (.filter(Optional::isPresent)
) and then converting those numbers to String (map(opt -> String.valueOf(opt.get()))
) and then joining all those strings to a single result (.collect(Collectors.joining("\n"));
).
Converting the input to a stream of string is a little fiddly, but worth it:
scanner.useDelimiter("\\s*\\n");
int count = Integer.parseInt(scanner.next());
// convert STDIN lines to a stream
Stream<String> commands = StreamSupport.stream(Spliterators.spliterator(scanner, count, Spliterator.ORDERED), false);
I ran that through the hackerranck submission and it scored 20 (full marks). The full code (using Stacker
instead of Solution
class name as required in hackerrank) is:
import java.io.StringReader;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.Optional;
import java.util.Scanner;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
public class Stacker {
private final Deque<Integer> stack = new ArrayDeque<>();
public Optional<Integer> processOperation(String op) {
char opc = op.charAt(0);
switch(opc) {
case '1':
int previous = stack.isEmpty() ? Integer.MIN_VALUE : stack.peek();
int max = Math.max(previous, Integer.parseInt(op.substring(1).trim()));
stack.push(max);
return Optional.empty();
case '2':
if (stack.isEmpty()) {
throw new IllegalStateException("Cannot pop empty stack");
}
stack.pop();
return Optional.empty();
case '3':
if (stack.isEmpty()) {
throw new IllegalStateException("Cannot max empty stack");
}
return Optional.of(stack.peek());
default:
throw new IllegalStateException("Unknown operation:" + op);
}
}
public static void main(String[] args) {
String testdata = "10\n" +
"1 97\n" +
"2\n" +
"1 20\n" +
"2\n" +
"1 26\n" +
"1 20\n" +
"2\n" +
"3\n" +
"1 91\n" +
"3";
try (Scanner scanner = new Scanner(new StringReader(testdata))) {
scanner.useDelimiter("\\s*\\n");
int count = Integer.parseInt(scanner.next());
// convert STDIN lines to a stream
Stream<String> commands = StreamSupport.stream(Spliterators.spliterator(scanner, count, Spliterator.ORDERED), false);
// process the operations:
Stacker stacker = new Stacker();
String report = commands.map(stacker::processOperation)
.filter(Optional::isPresent)
.map(opt -> String.valueOf(opt.get()))
.collect(Collectors.joining("\n"));
System.out.println(report);
}
}
}
You can see it in ideone here: https://ideone.com/1jREos
if (op.startsWith("1")){... }else if(op.equalsIgnoreCase("2")){...}else if(op.equals("3")){...}
.equalsIgnoreCase()
seems very strange - were you expecting upper-case digits? ;-) It would be neater and more readable if you used the same test for all three branches, or even simplyswitch
on the first character ofop
. \$\endgroup\$