12
\$\begingroup\$

In the spirit of April 2015 Community Challenge, I have created two Java-based calculators with the following codebase layout:

  • An interface Calculeightor for describing how values can be appended into a stream and then reduced to a single result given an operator.
  • An enum Operator to represent the operators.
  • The GUI implementation CalculeightorGUI.
  • The CLI implementation CalculeightorCLI.
  • A unit test for the CLI implementation CalculeightorCommandLineInterfaceTest.

The name uses eight as a pun, since this is written with Java 8 features in mind. Also, the unit test spells out CLI in full because appending Test to that creates another word that some may find inappropriate...

Without further ado, what follows are the code for Operator, CalculeightorCLI and CalculeightorCommandLineInterfaceTest. For the review of the other code, please look at the other review.

General feedback I am looking for:

  1. Any places where I can simplify the logic?
  2. Are Javadocs clear and concise enough?
  3. Any major faults?

Operator

public enum Operator implements BinaryOperator<Double> {
    ADD {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(t.doubleValue() + u.doubleValue());
        }

        @Override
        public String toString() {
            return "+";
        }
    },
    SUBTRACT {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(t.doubleValue() - u.doubleValue());
        }

        @Override
        public String toString() {
            return "-";
        }
    },
    MULTIPLY {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(t.doubleValue() * u.doubleValue());
        }

        @Override
        public String toString() {
            return "*";
        }
    },
    DIVIDE {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(t.doubleValue() / u.doubleValue());
        }

        @Override
        public String toString() {
            return "/";
        }
    },
    MODULUS {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(t.intValue() % u.intValue());
        }

        @Override
        public String toString() {
            return "%";
        }
    },
    EXPONENT {
        @Override
        public Double apply(Double t, Double u) {
            return Double.valueOf(Math.pow(t.doubleValue(), u.doubleValue()));
        }

        @Override
        public String toString() {
            return "^";
        }
    };

    static final Map<String, Operator> SYMBOLS = Stream.of(values()).collect(
            Collectors.toMap(Object::toString, Function.identity()));

    static Operator of(String operator) {
        return SYMBOLS.get(operator);
    }
}

CalculeightorCLI

/**
 * The CLI implementation of {@link Calculeightor}.
 * <p>
 * There is no limitation placed on the number of
 * inputs before entering the operator to use.
 */
public class CalculeightorCLI implements Calculeightor<Double> {

    public static final Map<String, Operator> OPERATORS = getOperators();
    public static final String RESULT_PREFIX = "Result: ";
    public static final String END_INPUT = "e";
    private static final int MIN_VALUES = 2;

    private final List<Double> inputs = new ArrayList<>();
    private BinaryOperator<Double> operator;

    private static Map<String, Operator> getOperators() {
        return EnumSet.complementOf(EnumSet.of(Operator.DIVIDE, Operator.MODULUS))
                .stream()
                .collect(Collectors.toMap(Object::toString, Function.identity()));
    }

    public void calculate(InputStream source, OutputStream out) {
        try (Scanner scanner = new Scanner(source);
                @SuppressWarnings("resource")
                PrintStream printer = new PrintStream(out)) {
            while (scanner.hasNext()) {
                String input = scanner.next().trim();
                if (endInput(input)) {
                    return;
                } else if (isOperator(input)) {
                    if (acceptOperator(input)) {
                        setOperator(OPERATORS.get(input));
                        showOutput(printer);
                    }
                } else {
                    acceptValue(input);
                }
            }
        }
    }

    private boolean endInput(String input) {
        return input.endsWith(END_INPUT);
    }

    private boolean isOperator(String input) {
        return OPERATORS.containsKey(input);
    }

    private boolean acceptOperator(String input) {
        if (inputs.size() < MIN_VALUES) {
            System.err.printf("Minimum %d integers before calculation.%n",
                    Integer.valueOf(MIN_VALUES));
            return false;
        }
        return true;
    }

    private void acceptValue(String input) {
        try {
            appendValue(Double.valueOf(Integer.parseInt(input)));
        } catch (NumberFormatException e) {
            System.err.println("Not an integer, ignored: " + input);
        }
    }

    /**
     * Writes the result onto the {@link OutputStream} via a {@link PrintStream}, and
     * then clear the inputs.
     *
     * @param out the {@link OutputStream} to use
     */
    private void showOutput(PrintStream out) {
        if (operator == null) {
            return;
        }
        out.println(RESULT_PREFIX + display(getResult()));
        inputs.clear();
    }

    @Override
    public void appendValue(Double value) {
        inputs.add(value);
    }

    @Override
    public Stream<Double> getValues() {
        return inputs.stream();
    }

    @Override
    public void setOperator(BinaryOperator<Double> operator) {
        this.operator = operator;
    }

    @Override
    public BinaryOperator<Double> getOperator() {
        return operator;
    }

    public static void main(String[] args) {
        System.out.printf("Enter integers, any of %s as operators or '%s' to exit.%n",
                OPERATORS.keySet(), END_INPUT);
        new CalculeightorCLI().calculate(System.in, System.out);
    }

}

CalculeightorCommandLineInterfaceTest

public class CalculeightorCommandLineInterfaceTest {

    enum TestCase {
        EXIT,
        INVALID_INPUTS("abc", "0.1", "0.0"),
        ONE_INTEGER("0"),
        ONE_INTEGER_AND_OPERATOR("0", "+"),
        ADD(Arrays.asList("6"), "1", "2", "3", "+"),
        SUBTRACT(Arrays.asList("-1"), "4", "5", "-"),
        MULTIPLY(Arrays.asList("42"), "6", "7", "*"),
        EXPONENT(Arrays.asList("134217728"), "8", "9", "^"),
        MULTI(Arrays.asList("21", "156"), "10", "11", "+", "-12", "-13", "*");

        private final List<String> expected;
        private final String[] inputs;

        private TestCase(String... inputs) {
            this(Collections.emptyList(), inputs);
        }

        private TestCase(List<String> expected, String... inputs) {
            this.expected = expected;
            this.inputs = inputs;
        }

        void doTest() {
            OutputStream out = new ByteArrayOutputStream();
            new CalculeightorCLI().calculate(toInputStream(inputs), out);
            assertThat(parseOutput(out), equalTo(expected));
        }

        /**
         * @param inputs the inputs to use
         * @return an {@link InputStream} of the inputs
         */
        private static InputStream toInputStream(String... inputs) {
            return new ByteArrayInputStream(Arrays.stream(inputs)
                    .collect(Collectors.joining("\n"))
                    .getBytes(StandardCharsets.UTF_8));
        }

        /**
         * @param out the output to parse
         * @return the {@link List} of calculation results
         */
        private static List<String> parseOutput(OutputStream out) {
            int prefix = CalculeightorCLI.RESULT_PREFIX.length();
            return Pattern.compile("\\v+").splitAsStream(out.toString().trim())
                    .map(i -> i.substring(prefix)).collect(Collectors.toList());
        }
    }

    @DataProvider(name = "test-cases")
    public Iterator<Object[]> getTestCases() {
        return Stream.of(TestCase.values()).map(t -> new Object[] { t }).iterator();
    }

    @Test(dataProvider = "test-cases")
    public void test(TestCase testCase) {
        testCase.doTest();
    }
}
\$\endgroup\$
5
\$\begingroup\$

enum improvements

I would use a constructor for your enum to make things more consise.

I would also use the DoubleBinaryOperator instead, which works directly with the primitive double type.

Additionally, you might as well make your Operator of(String operator) method public and your SYMBOLS map private.

Applying these changes will make your enum look like this:

public enum Operator implements DoubleBinaryOperator {
    ADD("+", (a, b) -> a + b),
    SUBTRACT("-", (a, b) -> a - b),
    MULTIPLY("*", (a, b) -> a * b),
    DIVIDE("+", (a, b) -> a / b),
    MODULUS("%", (a, b) -> a % b),
    EXPONENT("^", (a, b) -> Math.pow(a, b));

    private final String name;
    private final DoubleBinaryOperator op;

    private Operator(String name, DoubleBinaryOperator op) {
        this.name = name;
        this.op = op;
    }

    @Override
    public double applyAsDouble(double left, double right) {
        return op.applyAsDouble(left, right);
    }

    private static final Map<String, Operator> SYMBOLS = Stream.of(values()).collect(
        Collectors.toMap(Object::toString, Function.identity()));


    public static Operator of(String operator) {
        return SYMBOLS.get(operator);
    }

    @Override
    public String toString() {
        return name;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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