Lambdas will guide you home, CSS will ignite your code, Java I'll try to FX you

I've been learning JavaFX and wanted to recreate Simple calculator implemented with lambdas. It's also my first time using CSS, whose capability has convinced me of the wonders and appeal of JavaFX over swing.

Hitherto, I've only somewhat used swing and jumping into JavaFX this program is now my most complex, I like how it came out and would really appreciate a general review on anything I may be doing wrong/inefficiently/unconventionally.

The ultimate goal here would be of course to have it be a standalone jar.

Main class:

```
import javafx.application.Application;
import javafx.scene.Group;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.control.ComboBox;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.HBox;
import javafx.scene.paint.Color;
import javafx.stage.Stage;
import javafx.stage.StageStyle;

public class Calculator extends Application {
public static void main(String[] args) {
launch(args);
}

@Override
public void start(Stage stage) {
stage.setTitle("Thank you CodeReview");

ComboBox operations = new ComboBox<>();
for (Operation op : Operation.values()) {
operations.getItems().add(op);
}
operations.setValue(Operation.values()[0]);

TextField[] input = new TextField[2];
for (int i = 0; i result.setText(
computeResult(
(Operation)operations.getValue(),
input[0].getText(),
input[1].getText()
)
)
);

HBox box = new HBox();
box.getChildren().addAll(
input[0], operations, input[1], calculate, result
);

Group root = new Group();
root.getChildren().add(box);

Scene scene = new Scene(root);
scene.getStylesheets().add("Calculator.css");
scene.setFill(Color.WHITE);

stage.initStyle(StageStyl

Summary

• Lambda in enum

• Maybe use java.util.function.BinaryOperator

• Law of Demeter

• Shorter methods

• Extensibility

• CSS

• Collections addAll()

Lambda in enum

Now this is really cool stuff. I love reading the Operation enum over and over again. It is beautiful.

java.util.function.BinaryOperator

Your definition of interface Equation seems redundant with BinaryOperator. Maybe you want to use BinaryOperator instead of your own interface Equation.

Law of Demeter

Your enum Operation forces the caller (in this case it's computeResult()) to violate the Law of Demeter.

You could instead:

public enum Operation implements Equation {
    ADD((x, y) -> x + y),
    // ...
    private final Equation equation;
    // ...
    public double compute(final double val1, final double val2) {
        return equation.compute(val1, val2);
    }
}

Then a call to getEquation() would no longer be required, the Operation could be used directly because it's an Equation itself. (Or BinaryOperator if you go for that.)

Shorter methods

start() is too long, I'd split it.
You will have to move some of the variables into fields for that, and I think that's okay.
It also comes at another benefit, you could use a method reference instead of a lambda for calculate.setOnAction() then.

Extensibility

The disadvantage of an enum is that you cannot add new values at runtime. With that respect, an enum is somewhat a switch-case in a very smart disguise. It comes with all the OO stuff and polymorphism, but when you want new elements, you still have to change the original source. But it's easy to break that. You could use a Set which you initialize with addAll(Operation.values()) (requires that Operation implements Equation which is recommended for the Law of Demeter anyway).

Not sure whether you really want to make that a requirement for your app, I just thought it might be interesting to discuss.

CSS

In case two selectors have intentionally the same properties, you could make that more obvious by joining their declarations. For example, you could join

.combo-box .cell:hover {
    -fx-font-weight: bold;
    -fx-text-fill: white;
    -fx-background-color: calculator-base;
}
.combo-box .cell:focused {
    -fx-font-weight: bold;
    -fx-text-fill: white;
    -fx-background-color: calculator-base;
}

into

.combo-box .cell:hover, .combo-box .cell:focused {
    -fx-font-weight: bold;
    -fx-text-fill: white;
    -fx-background-color: calculator-base;
}

You can also have multiple rules for the same selector. With CSS you're always in a dilemma: Do you go for selector-ordering or for property-ordering... I tend to follow a mixed approach, trying to avoid redundancy as much as possible.

If your CSS grows bigger and you tend to use the same property values in different places, you might want to go for a CSS preprocessor in order to replace those values with preprocessor constants.
This can significantly increase the maintainability of non-trivial CSS.

Collections addAll()

I think your code

ComboBox operations = new ComboBox<>();
for (Operation op : Operation.values()) {
    operations.getItems().add(op);
}

could be simplified to

ComboBox operations = new ComboBox<>();
operations.getItems().addAll(EnumSet.allOf(Operation.class));

If the API can do something for me, I tend to delegate things to the API.

StackExchange Code Review Q#83477, answer score: 3