Implementation of stack

Task:


Create a class RPNStack which represents a stack of objects of type
Node. Class RPNStack contains only one private field top of type Node.
Objects of type Node represent data that are pushed on the stack: each
object contains in its field val a double and in field next a reference to
the next node (as in a singly-linked list - top plays here the role of
the head ). Class RPNStack offers three methods:

• method public void push(double d) pushing new object of type Node on top of the stack (i.e., it becomes the new top);

• method public double pop() removing the top node (so the next node becomes the new top) and returning val from the removed node;

• method public boolean empty() returning true if and only if the stack is empty (top is null); otherwise false is returned.

Note that stack is a singly-linked list where adding and removing
elements is always performed at the beginning.


The main program reads a le with data representing arithmetic
expressions in the Reverse Polish Notation (RPN), for example:


2 7 5 + * 20 - 1 4 / /


After reading each line, it is split (using spaces as separators) and
for each token:

• if it is string "+", we pop two numbers from the stack, add them and push the result on the stack;

• if it is string "*", we do the same but myltiplying the numbers instead of adding them;

• if it is string "-", we pop two elements, subtract the one popped as the first from the one popped as the second and push the result on the stack;

• if it is string "/", we do the same but dividing the numbers instead of subtracting them;

• if it is not "+", "*", "-" or "/", we interpret it as a number of type double and we push it onto the stack.

After all tokens from the line have been processed, we pop the
remaining number o the stack, which should be the value of the whole
expression. We then print the line and the result. We also check if
the stack is now empty;

Static methods on RPNStack

It is odd to write a class like RPNStack with all static methods and fields; effectively, you have a single global stack, for no particularly good reason.

The obvious thing to do would be to make all of those methods and fields instance methods/fields, and then create an instance of RPNStack within main().

Output

It isn't clear to me why "\n" would not be working for you where System.lineSeparator() does; however, you aren't closing or flushing the file, and it may be the case that some subtle difference is causing the file to be flushed in one case and not in the other.

I would expect that if you close your BufferedWriter at the end of your program, it will work fine.

Recognising symbols

The obvious and lightweight way to recognise a small set of symbols like this would be to use a switch statement. If you have Java 7+ (which you really should) you can just use strings directly as case labels; otherwise, you'd have to switch on characters (which would still work for your simple language). For a more complex language, you could consider a map from symbols to actions.

Error

There are kinds of errors which won't be caught by your catch (IOException). But I don't think you need to worry too much about those.

The only bit of error handling I would consider adding is that it's possible for the user to enter an expression which causes a divide by zero error. As your program stands, this will throw an ArithmeticException and terminate the program. I would suggest either catching the ArithmeticException and recovering with an error message, or validating the operands you're passing to the division operator.

Minor nitpicking

• Conventionally, you would print error messages to System.err rather than System.out.

-
The RPNStack.empty() method could be more clearly written by just directly returning the value of the boolean expression:

public boolean empty() { return top == null; }

-
You don't need a special case for the first node on a stack: if you just do

top = new Node(d, top);

that will have the correct behaviour whether top points to a Node or is null.

-
Consider checking whether the stack is empty when popping it; this will let you print a useful error message rather than dying with a null pointer exception.

StackExchange Code Review Q#119521, answer score: 5