3
\$\begingroup\$

Today I worked on a JavaFX project about fleet management and wondered what else I could add or improve. Unfortunately, the code is written in German, as are any comments. I hope that's not a problem. I look forward to your input. And yes, I'm new here and also new to programming, and I don't know much about JavaFX yet. Now I thought I'd take the opportunity to do two things at once: get to know Stackoverflow and maybe improve my code's performance, as well as get ideas for possible extensions.

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.geometry.*?>
<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>
<?import javafx.scene.text.*?>

<VBox alignment="CENTER" prefHeight="391.0" prefWidth="451.0" spacing="20.0" xmlns="http://javafx.com/javafx/17.0.12" xmlns:fx="http://javafx.com/fxml/1" fx:controller="fuhrparkverwaltung.FahrzeugController" stylesheets="@style.css">
    <padding>
        <Insets bottom="20.0" left="20.0" right="20.0" top="20.0" />
    </padding>
    <children>
        <Label text="Fuhrparkverwaltung">
         <font>
            <Font name="System Bold" size="18.0" />
         </font></Label>
        <GridPane prefHeight="170.0" prefWidth="411.0">
            <columnConstraints>
                <ColumnConstraints hgrow="SOMETIMES" maxWidth="201.0" minWidth="10.0" prefWidth="136.0" />
                <ColumnConstraints hgrow="SOMETIMES" maxWidth="323.0" minWidth="10.0" prefWidth="276.0" />
            </columnConstraints>
            <rowConstraints>
                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
                <RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
            </rowConstraints>
            <children>
                <Label text="Kennzeichen" />
                <Label text="Marke" GridPane.rowIndex="1" />
                <Label text="Kilometerstand" GridPane.rowIndex="2" />
                <Label text="Maximale Ladung" GridPane.rowIndex="3" />
                <TextField fx:id="txtkennzeichen" GridPane.columnIndex="1" />
                <TextField fx:id="txtmarke" GridPane.columnIndex="1" GridPane.rowIndex="1" />
                <TextField fx:id="txtkilometer" GridPane.columnIndex="1" GridPane.rowIndex="2" />
                <CheckBox fx:id="ckdAnhaenger" mnemonicParsing="false" GridPane.columnIndex="1" GridPane.rowIndex="4" />
                <Label text="Anhängerkupplung" GridPane.rowIndex="4" />
                <TextField fx:id="txtmaxladung" GridPane.columnIndex="1" GridPane.rowIndex="3" />
            </children>
        </GridPane>
        <HBox prefHeight="100.0" prefWidth="200.0" spacing="40.0">
            <children>
                <Button onAction="#addFahrzeug" text="Fahrzeug hinzufügen" />
                <Button mnemonicParsing="false" onAction="#sortFahrzeug" text="Sortierung nach Kilometerstand" />
                <Button mnemonicParsing="false" onAction="#clearFields" text="Felder leeren" />
            </children>
            <VBox.margin>
                <Insets left="140.0" />
            </VBox.margin>
        </HBox>
        <ListView fx:id="fahrzeugList" prefHeight="200.0" prefWidth="200.0" />
    </children>
</VBox>

import javafx.application.Application;

public class PKW extends Fahrzeug {

    private boolean _hatAngängerkupplung;

    public PKW(String kennzeichen, String marke, int kmStand, boolean hatAngängerkupplung) {
        super(kennzeichen, marke, kmStand);
        _hatAngängerkupplung = hatAngängerkupplung;
    }
    @Override
    public String toString() {
        String s = "PKW: Kennzeichen: " + getKennzeichen() + ", Marke: " + getMarke() + ", KmStand: " + getKmStand() + ", Anhängerkupplung: ";
        if(_hatAngängerkupplung){
            s += "Ja";
        }
        else {
            s += "Nein";
        }
        return s;
    }
}

package at.htl.fuhrparkverwaltung;

public class LKW extends Fahrzeug{

    private double _maxZuladung;

    public LKW(String kennzeichen, String marke, int kmStand, double _maxZuladung) {
        super(kennzeichen, marke, kmStand);
        this._maxZuladung = _maxZuladung;
    }

    @Override
    public String toString() {
        return  "LKW: Kennzeichen: " + getKennzeichen() + ", Marke: " + getMarke() + ", KmStand: " + getKmStand() + ", Maximale Zuladung" + _maxZuladung;
    }
}

package at.htl.fuhrparkverwaltung;

public abstract class Fahrzeug {

    private String _kennzeichen;
    private String _marke;
    private int _kmStand;

    public Fahrzeug(String kennzeichen, String marke, int kmStand) {
        _kennzeichen = kennzeichen;
        _marke = marke;
        _kmStand = kmStand;
    }
    public String getKennzeichen() {
        return _kennzeichen;
    }
    public String getMarke() {
        return _marke;
    }
    public int getKmStand() {
        return _kmStand;
    }

    public abstract String toString();


}

package at.htl.fuhrparkverwaltung;

import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.event.ActionEvent;
import javafx.scene.control.*;

public class FahrzeugController {

    public TextField txtkennzeichen;
    public TextField txtmarke;
    public TextField txtkilometer;
    public CheckBox ckdAnhaenger;
    public TextField txtmaxladung;
    public ListView fahrzeugList;

    private ObservableList<Fahrzeug> fahrzeuge = FXCollections.observableArrayList();

    public void initialize() {
        fahrzeugList.setItems(fahrzeuge);

        fahrzeuge.add(new LKW("JO-911-YI", "Ford", 91111, 92.2));
        fahrzeuge.add(new LKW("JO-123-AH", "KIA", 12313, 12.12));
        fahrzeuge.add(new PKW("JO-721-VA", "Aston", 15512, true));
        fahrzeuge.add(new PKW("ZE-521-GI", "Samia", 102111, false));

        txtmaxladung.textProperty().addListener((observable, oldValue, newValue) -> {
            if (!newValue.isEmpty()) {
                ckdAnhaenger.setDisable(true);
                ckdAnhaenger.setSelected(false);
            } else {
                ckdAnhaenger.setDisable(false);
            }
        });
        ckdAnhaenger.selectedProperty().addListener((observable, oldValue, newValue) -> {
            if (newValue) {
                txtmaxladung.setDisable(true);
                txtmaxladung.clear();
            } else {
                txtmaxladung.setDisable(false);
            }


        });
    }

        public void addFahrzeug (ActionEvent actionEvent){
            String marke = txtmarke.getText().trim();
            String kennzeichen = txtkennzeichen.getText().trim();
            String kilometers = txtkilometer.getText().trim();
            String maxladung = txtmaxladung.getText().trim();
            boolean anhaenger = ckdAnhaenger.isSelected();

            String match = "^([A-Z]{1,2}\\-)((\\d){3}\\-([A-Z]{2})|((\\d){5}))$";

            if (!kennzeichen.matches(match)) {
                showError("Ungültiges Kennzeichen", "Das eingegebene Kennzeichen ist ungültig");
                return;
            }
            for (Fahrzeug fahrzeug : fahrzeuge) {
                if (kennzeichen.equals(fahrzeug.getKennzeichen())) {
                    showError("Doppeltes Kennzeichen", "Das eingegebene Kennzeichen ist bereits vergeben");
                    return;
                }
            }
            if (marke.isEmpty()) {
                showError("Keine Marke gefunden", "Keine Marke gefunden");
                return;
            }
            if (kilometers.isEmpty()) {
                showError("Keine Kilometer", "Keine Kilometer gefunden");
                return;
            }
            try {
                int kilometer = Integer.parseInt(kilometers);
                double ladung = Double.parseDouble(maxladung);
                if (!maxladung.isEmpty()) {
                    fahrzeuge.add(new LKW(kennzeichen, marke, kilometer, ladung));
                } else {
                    fahrzeuge.add(new PKW(kennzeichen, marke, kilometer, anhaenger));
                }

            } catch (NumberFormatException e) {
                showError("Falsches Zahlenformat eingegeben", e.getMessage());
            }


        }

        public void sortFahrzeug (ActionEvent actionEvent){
            fahrzeuge.sort((fahrzeug1, fahrzeug2) -> {
                return fahrzeug1.getKennzeichen().compareTo(fahrzeug2.getKennzeichen());
            });

        }

        public void clearFields (ActionEvent actionEvent){
            txtkennzeichen.clear();
            txtmarke.clear();
            txtkilometer.clear();
            ckdAnhaenger.setSelected(false);
            txtmaxladung.clear();
        }

        public void showError (String title, String message){
            Alert alert = new Alert(Alert.AlertType.WARNING);
            alert.setTitle(title);
            alert.setHeaderText(null);
            alert.setContentText(message);
            alert.showAndWait();
        }
    }

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

import java.io.IOException;

public class HelloApplication extends Application {
    @Override
    public void start(Stage stage) throws IOException {
        FXMLLoader fxmlLoader = new FXMLLoader(HelloApplication.class.getResource("hello-view.fxml"));
        Scene scene = new Scene(fxmlLoader.load(), 760, 540);
        stage.setTitle("Fuhrparkverwaltung");
        stage.getIcons().add(new Image(HelloApplication.class.getResourceAsStream("icon.png")));
        stage.setScene(scene);
        stage.show();
    }
}

package at.htl.fuhrparkverwaltung;

import javafx.application.Application;

public class Launcher {
    public static void main(String[] args) {
        Application.launch(HelloApplication.class, args);
    }
}
New contributor
user32091719 is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
0

2 Answers 2

Reset to default
3
\$\begingroup\$

A small style note: the following might benefit from the "ternary operator."

    @Override
    public String toString() {
        String s = "PKW: Kennzeichen: " + getKennzeichen() + ", Marke: " + getMarke() + ", KmStand: " + getKmStand() + ", Anhängerkupplung: ";
        if(_hatAngängerkupplung){
            s += "Ja";
        }
        else {
            s += "Nein";
        }
        return s;
    }

Can be:

    @Override
    public String toString() {
        String s = "PKW: Kennzeichen: " + getKennzeichen() + ", Marke: " + getMarke() + ", KmStand: " + getKmStand() + ", Anhängerkupplung: ";
        s += _hatAngängerkupplung ? "Ja" : "Nein";
        return s;
    }

But really, simpler:

    @Override
    public String toString() {
        return "PKW: Kennzeichen: " + getKennzeichen() + 
               ", Marke: " + getMarke() + ", KmStand: " + 
               getKmStand() + ", Anhängerkupplung: " +
               (_hatAngängerkupplung ? "Ja" : "Nein");
    }

You may also prefer to use String.format to express this more cleanly.

    @Override
    public String toString() {
        return String.format(
            "PKW: Kennzeichen: %s, Marke: %s, KmStand: %d, Anhängerkupplung: %s",
            getKennzeichen(), getMarke(), getKmStand(),
            _hatAngängerkupplung ? "Ja" : "Nein"
        );
    }
\$\endgroup\$
1
\$\begingroup\$

Caveat lector before this review, I don't know much (more) about JavaFX than you do.

Field, class and other names

As far as I'm aware off things, using anything other than english for class and field names is an extreme rarity. This applies particularely for Umlaute (äöüß), which external people working on your project might not even have on their keyboard. Also, if you do decide on a language, do it all the way through (Currently, FahrzeugController.showError(...) is english, for example).

  • hello-view.fxml -> fahrzeug-uebersicht-view.fxml (probbably a leftover from project creation)
  • Prefixing private fields with a _ is not done in Java. The private modifier usally already conveys to people that this member is internal and may change at any time without notice.
  • Based upon the current example values, Fahrzeug._marke is more a Hersteller, or Fabricator.

Probbable Bugs

  • The controller value specified in hello-view.fxml and the actual controller location from your question does not add up. (FXML was fuhrparkverwaltung.FahrzeugController, question is at.htl.fuhrparkverwaltung. I personally favor the latter.)
  • FahrzeugController.addFahrzeug checks for an empty maxladung after Double.parseDouble(...) has already thrown a NFE, making adding new PKW's impossible.

Class Design issiues

  • the regex for validating license plates is odd-placed in FahrzeugController.addFahrzeug. It should become a public static final class member somewhere (for now perhaps in Fahrzeug, although if you ever add an abstraction for license plates so you can support license plate formats from other countries, it might need to be rehomed again). Perhaps even make a Pattern member out of it, so it can be compiled once at class load time, rather than each and every time someone adds a new Vehicle.
  • As far as I'm aware off things, a maximale Zuladung is not entirely exclusive to Trucks, at least some PKW's have such a value too (although people seem to seldomly know, much less check, that value). Wether thats something you want to model depends on your buissness context, which I don't know. So wether you want to track the max load for PKW's is something you decide.
  • Similarely (although much less common, AFAIK only for specialist vehicles; I've seen it with Trucks used to carry mail between hubs, and sometimes with trucks used to deliver goods from a hub to a supermarket) it is not entirely unheard off for Trucks to have the ability to support an Anhänger. Again, wether that something you want to model is up to you.

Misc. Stuff

  • Please avoid star-imports, such as in FahrzeugController with javafx.scene.control.*. It makes it difficult to see what you're actually using and might produce weird results if more than one package contains a class by the same simple name.
  • PKW imports javafx.application.Application for no reason.

Possible extensions

  • add support for license plates from other countries.
  • Add serialisation support to the application, allowing people to store the vehicles they track.
  • add support for modifying tracked vehicles (particularely the KmStand has the potential to change over time)
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.