GUI #9
GUI #9
Conversation
While resolving merge conflict forgot to check if genotypeLength exists. Made Genotype abstract and divided mutation method into 2 seperate classes.
| package agh.ics.oop; | ||
|
|
||
| import javafx.application.Application; | ||
|
|
||
| public class World { | ||
| public static void main(String[] args) { | ||
| Application.launch(SimulationApp.class, args); | ||
| } | ||
| } |
There was a problem hiding this comment.
the App should be launched from here by running World class
src/main/resources/simulation.fxml
Outdated
| <?import javafx.scene.layout.BorderPane?> | ||
| <BorderPane xmlns="http://javafx.com/javafx" | ||
| xmlns:fx="http://javafx.com/fxml" | ||
| minHeight="550.0" minWidth="850.0" | ||
| fx:controller="agh.ics.oop.presenter.SimulationPresenter"> | ||
| minHeight="550.0" minWidth="850.0"> | ||
| <center> | ||
| <Label fx:id="infoLabel" text="All animals will be living here!" textAlignment="CENTER"/> | ||
| </center> |
There was a problem hiding this comment.
this file will be showing the simulation in some time
| public void initialize() { | ||
| mapWidth.setTextFormatter(positiveInteger()); | ||
| mapHeight.setTextFormatter(positiveInteger()); | ||
| mapWidth.textProperty().addListener((observable, oldValue, newValue) -> { |
There was a problem hiding this comment.
Should values be validated on every change or only after startTheSimulation Button changed?
Right now if you try to change values you have to do it in the correct order and it probably isn't a good UX.
| int height = getValueFromTextField(mapHeight); | ||
|
|
||
| checkAndClearIfGreaterThan(initialNumberOfPlants, width * height); | ||
| checkAndClearIfGreaterThan(initialNumberOfAnimals, width * height); |
There was a problem hiding this comment.
Many animals can be on one tile. So unless it is a restriction to prevent lagging and user overuse we can delete this line.
If you decide to do so don't forget to delete line 235 as well.
| initialEnergyOfAnimals.setTextFormatter(positiveInteger()); | ||
| energyToBeWellFed.setTextFormatter(positiveInteger()); | ||
| energyToBeWellFed.textProperty().addListener((observable, oldValue, newValue) -> { | ||
| setTextFormatterLessThan(energyToBeWellFed, energyToReproduce); |
There was a problem hiding this comment.
name of this method and setTextFormatterLessThanWholeMap is a bit misleading because if the values are equal it still works. You can change ...Less... to ...NotGreater... or something else to make the names shorter if possible.
| alert.showAndWait(); | ||
| } | ||
|
|
||
| private void setTextFormatterLessThan(TextField maxValue, TextField lowerValue) { |
There was a problem hiding this comment.
What exacly is maxValue and lowerValue?
If I understand it correctly lowerValue is value to check and max is the mac limit set?
If so the variables names are chosen poorly.
| }); | ||
| } | ||
|
|
||
| private TextFormatter<Integer> positiveInteger() { |
There was a problem hiding this comment.
Previously we allowed some values to be equal to 0. For instance:
- initialNumberOfPlants
- energyFromOnePlant;
- plantsEachDay;
- minNumberOfMutations;
- maxNumberOfMutations;
I only mentioned those that would have sense if we decide to let them be 0 again.
Current state is not bad, but I want to make sure that in previous code there are usages that will need to be changed. Fe. in GenotypeTests.
|
|
||
| ObservableList<String> optionsOfMap = FXCollections.observableArrayList( | ||
| "Underground tunnels", | ||
| "Rectangular map" |
There was a problem hiding this comment.
I would still to the "Globe" name.
| && !mapHeight.getText().isEmpty() | ||
| && !initialNumberOfPlants.getText().isEmpty() | ||
| && !energyFromOnePlant.getText().isEmpty() | ||
| && !initialNumberOfAnimals.getText().isEmpty() | ||
| && !plantsEachDay.getText().isEmpty() | ||
| && !initialEnergyOfAnimals.getText().isEmpty() | ||
| && !energyToBeWellFed.getText().isEmpty() | ||
| && !energyToReproduce.getText().isEmpty() | ||
| && !lengthOfGenotypes.getText().isEmpty() | ||
| && !maxNumberOfMutations.getText().isEmpty() | ||
| && !minNumberOfMutations.getText().isEmpty() |
There was a problem hiding this comment.
All those TextFields can be stored in a List. Then instead of writing everything by hand you can iterate through this list. You could also iterate through this list while setting positiveInteger TextFormatter in initialize().
It would look something like that:
private boolean areAllValuesFilled() {
List<TextField> fields = Arrays.asList(
mapWidth, mapHeight, initialNumberOfPlants, energyFromOnePlant,
initialNumberOfAnimals, plantsEachDay, initialEnergyOfAnimals,
energyToBeWellFed, energyToReproduce, lengthOfGenotypes,
maxNumberOfMutations, minNumberOfMutations
);
return fields.stream().allMatch(field -> !field.getText().isEmpty())
&& mutationOption.getValue() != null
&& mapOption.getValue() != null;
}
| try { | ||
| int newValue = Integer.parseInt(newText); | ||
|
|
||
| if (newValue < max) { |
There was a problem hiding this comment.
Do we want to allow some values to be the same? For instance minNumberOfmutations and maxNumberOfMutations. If not you have to make sure that nowhere else is the code we are allowing that to happen.
Deleted redundant class AnimalTree and changed some tests.
Minor fixes
No description provided.