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

Postfix calculator

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

Problem

As part of my learning Java I'm doing various tasks I set myself - one such task is a postfix calculator. It seems to work with all tests I've given it, and I would really appreciate some review of the code I've created to complete this, both logic and style wise:

```
package com.VortixDev.PostfixInterpreter;

import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class PostfixInterpreter {
public static void main(String[] args) {
Scanner scan = new Scanner(System.in);

String postfix = scan.nextLine();

scan.close();

postfix = calculate(postfix);

System.out.println(postfix);
}

public static String calculate(String postfix) {
String result = postfix;
String initialPostfix;

do {
initialPostfix = result;

do {
postfix = result;

result = processPostfix(result, '/');
} while (result != postfix);

do {
postfix = result;

result = processPostfix(result, '*');
} while (result != postfix);

do {
postfix = result;

result = processPostfix(result, '+');
} while (result != postfix);

do {
postfix = result;

result = processPostfix(result, '-');
} while (result != postfix);
} while (initialPostfix != result);

return result;
}

private static String processPostfix(String postfix, char operator) {
String numberMatch = "(\\-?\\d+(?:\\.\\d+)?)";

Matcher matcher = Pattern.compile(numberMatch + " " + numberMatch + " \\" + operator).matcher(postfix);

boolean findMatch = matcher.find();

while (findMatch) {
String match = matcher.group(0);

double firstValue = Double.parseDouble(matcher.group(1));
double secondValue = Double.parseDouble(m

Solution

Too much whitespace (particularly empty lines) for my liking. Try to use empty lines to group code into small clusters of code that belong together. For example, you wrote:

switch (operator) {
    case '/':
        resultValue = firstValue / secondValue;

    break;
    case '*':
        resultValue = firstValue * secondValue;

        break;


Whereas, imo, it would be easier to read like this:

switch (operator) {
    case '/':
        resultValue = firstValue / secondValue;
        break;

    case '*':
        resultValue = firstValue * secondValue;
        break;

Code Snippets

switch (operator) {
    case '/':
        resultValue = firstValue / secondValue;

    break;
    case '*':
        resultValue = firstValue * secondValue;

        break;
switch (operator) {
    case '/':
        resultValue = firstValue / secondValue;
        break;

    case '*':
        resultValue = firstValue * secondValue;
        break;

Context

StackExchange Code Review Q#138894, answer score: 2

Revisions (0)

No revisions yet.