Numbers are high, numbers are low. Will you guess the right answer, though?

Question

This question is a follow-up to:

High-Low Guessing Game

• Now with shiny new graphics in JavaFX.
• You now have the awesome ability to choose which numbers to guess between.
• Now with Git Repository!

Main.java

package sample;

import javafx.application.Application;
import javafx.fxml.FXMLLoader;
import javafx.scene.Parent;
import javafx.scene.Scene;
import javafx.stage.Stage;

public class Main extends Application {

    @Override public void start(Stage stage) throws Exception {
        Parent root = FXMLLoader.load(getClass().getResource("highlow_view.fxml"));
        Scene scene = new Scene(root);

        stage.setScene(scene);
        stage.show();
    }


    public static void main(String[] args) {
        launch(args);
    }
}

Controller.java

package sample;

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.input.KeyCode;
import javafx.scene.input.KeyEvent;
import java.util.Random;

public class Controller {
    @FXML private Label guessLabel;
    @FXML private Label numberGuessLabel;
    @FXML private TextField txtAddItem;
    @FXML private TextField fromAddItem;
    @FXML private TextField toAddItem;
    @FXML private Label systemOut;
    private static Random rand = new Random();
    private boolean isInt;
    public int randomNumberFrom = 1;
    public int randomNumberTo = 10;
    private int numberGuess = 0;
    private int theRandomNumber = randomInt(randomNumberFrom, randomNumberTo);

    public int getTheRandomNumber() {
        return theRandomNumber;
    }

    public static int randomInt(int from, int to) {
        return rand.nextInt(to - from + 1) + from;
    }

    public void countGuess(int numberIn) {
        numberGuess++;
        numberGuessLabel.setText("Guess counter:" + numberGuess);
        if (numberIn < theRandomNumber) {
            systemOut.setText("Too low");
        } else if (numberIn > theRandomNumber) {
            systemOut.setText("Too high");
        } else {
            systemOut.setText("Congratz you were right!");
        }
    }

    public int textInt(TextField textField) {
        if(!textField.getText().matches("^\\d+$")) {
            systemOut.setText("Pleas enter a number");
            isInt = false;
            return 0;
        }else{
            isInt = true;
            return Integer.parseInt(textField.getText());
        }
    }

    public void guessing(int numberIn){
        if (isInt == true) {
            if (numberIn < randomNumberFrom || numberIn > randomNumberTo) {
                systemOut.setText("Pleas enter number: " + randomNumberFrom + "-" + randomNumberTo);
            } else {
                countGuess(numberIn);
            }
        }
    }

    @FXML public void getNewNumber(ActionEvent action){
        int fromInt = textInt(fromAddItem);
        if (isInt == true) {
            int toInt = textInt(toAddItem);
            if (isInt == true && fromInt < toInt) {
                numberGuess = 0;
                numberGuessLabel.setText("Guess counter:" + numberGuess);
                theRandomNumber = randomInt(fromInt, toInt);
                randomNumberFrom = fromInt;
                randomNumberTo = toInt;
                guessLabel.setText("Guess a number:" + fromInt + "-" + toInt);
            } else{
                systemOut.setText("The second number must be bigger then the first");
            }
        }
    }

    @FXML private void handleEnterPressed(KeyEvent event) {
        if (event.getCode() == KeyCode.ENTER) {
            guessing(textInt(txtAddItem));
        }
    }

    @FXML private void guessButton(ActionEvent action){
        guessing(textInt(txtAddItem));
    }
}

highlow_view.fxml

<?xml version="1.0" encoding="UTF-8"?>
<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>
<?import javafx.scene.text.*?>

<AnchorPane id="AnchorPane" prefHeight="200.0" prefWidth="500.0" xmlns:fx="http://javafx.com/fxml" fx:controller="sample.Controller">
<children>
    <VBox fx:id="VBoxMain" alignment="TOP_CENTER" prefHeight="500.0" prefWidth="400.0" AnchorPane.bottomAnchor="40.0" AnchorPane.leftAnchor="100.0" AnchorPane.rightAnchor="100.0" AnchorPane.topAnchor="20.0">
        <children>
            <Label fx:id="systemOut" text="Hello :D" alignment="CENTER" prefHeight="200.0">
                <font>
                    <Font name="System Bold" size="18.0" />
                </font>
            </Label>
            <Label  fx:id="numberGuessLabel" text="Guess counter:0" />
            <Label  fx:id="guessLabel" text="Guess a number:1-10" />
            <TextField fx:id="txtAddItem"  prefWidth="200.0" onKeyPressed="#handleEnterPressed" />
            <HBox id="HBox" fx:id="HBox4Btns" alignment="CENTER" spacing="5.0">
                <children>
                    <Button mnemonicParsing="false" onAction="#guessButton" text="Guess"/>
                    <Button mnemonicParsing="false" onAction="#getNewNumber" text="New Number"/>
                    <TextField fx:id="fromAddItem"  prefWidth="50.0"/>
                    <Label  fx:id="fromToo" text="-" />
                    <TextField fx:id="toAddItem"  prefWidth="50.0"/>
                </children>
            </HBox>
        </children>
    </VBox>
</children>
</AnchorPane>

Answer 1

private static Random rand = new Random();

This variable can/should be final

---

public int randomNumberFrom = 1;
public int randomNumberTo = 10;

In Java it is better practice, if you want to allow access to your variables, to make getters and setters and make the variables themselves private.

private int randomNumberFrom = 1;
private int randomNumberTo = 10;

public int getRandomNumberFrom() {
    return this.randomNumberFrom;
}

public int getRandomNumberTo() {
    return this.randomNumberTo;
}

In this case, since these values are so tightly tied together, you could have one method to set both values:

public void setRandomRange(int from, int to) {
    this.randomNumberFrom = from;
    this.randomNumberTo = to;
}

Ideally, you should also do some validation in this method to ensure that to > from. Like this:

public void setRandomRange(int from, int to) throws IllegalArgumentException {
    if (to <= from) {
        throw new IllegalArgumentException("to must be greater than from");
    }
    this.randomNumberFrom = from;
    this.randomNumberTo = to;
}

---

numberGuess++;
numberGuessLabel.setText("Guess counter:" + numberGuess);

This variable numberGuess has a bad name. It's not until I read this code that I realize "ok, it counts the number of guesses". It could just as well have been the number you guessed at. guessCount would be a better name.

---

Your textInt method smells because it modifies the isInt field. It also sets the error message in case it's not a valid number. And it also returns an int.

public int textInt(TextField textField) {
    if(!textField.getText().matches("^\\d+$")) {
        systemOut.setText("Pleas enter a number");
        isInt = false;
        return 0;
    }else{
        isInt = true;
        return Integer.parseInt(textField.getText());
    }
}

One small improvement, because you are not using return values less than 0, is to return -1 as a "special case" to indicate that it is not a valid integer. This would get rid of the isInt variable as a start

public int textInt(TextField textField) {
    if(!textField.getText().matches("^\\d+$")) {
        systemOut.setText("Pleas enter a number");
        return -1;
    }else{
        return Integer.parseInt(textField.getText());
    }
}

Then you just check for value >= 0 instead of isInt.

However, it might be better to use the fact that Integer.parseInt throws a NumberFormatException to do this. I think it might be best if you use the following approach, without using it as a method:

try {
    int value = Integer.parseInt(textField.getText());
    // do something if value is integer
}
catch (NumberFormatException ex) {
    systemOut.setText("Please enter a number");
}

---

    @FXML private void handleEnterPressed(KeyEvent event) {
        if (event.getCode() == KeyCode.ENTER) {
            guessing(textInt(txtAddItem));
        }
    }


    @FXML private void guessButton(ActionEvent action){
        guessing(textInt(txtAddItem));
    }

In this case you can have one method call the other one:

    @FXML private void handleEnterPressed(KeyEvent event) {
        if (event.getCode() == KeyCode.ENTER) {
            guessButton(null);
        }
    }