Projektaufgabe: Restaurantbestellung - Codefeedback gewünscht

MoonScripter

Mitglied
Guten Abend zusammen,

ich bin gerade beim Entwickeln meines allerersten Java Projekts. Dieses wird ein Restaurantbestellungssystem werden, welches eine reine Konsolenanwendung ist.

Die Planung sowie Umsetzung hat mich vor gewisse Herausforderungen gestellt, die ich wie ich finde gut meistern konnte.

Ich habe das Projekt in 3 Models und 1 Controller aufgeteilt:

Models
- Customer
- Menu
- Order

Controller
- RestaurantPaymentApp

Dies schien für mich die beste Aufteilung nach den mir bekannten OOP-Regeln zu sein.

Ich bin mir bei der Entwicklung der Models und des Controllers unsicher, ob dies die richtige Umsetzung ist oder ob ich da doch das ein und andere vermische. Daher möchte ich hier gerne meinen Code teilen und euch fragen, wie ihr es sieht. Ich würde mich sehr auf euer Codefeedback freuen.

Hier ist der Code:
[CODE lang="java" title="Model: Customer"]package model;

public class Customer {

private String firstName;
private String lastName;
private String deliveryAddress;

public Customer() {

}

public void setFirstName(String firstName) {
this.firstName = firstName;
}

public void setLastName(String lastName) {
this.lastName = lastName;
}

public String getFullName() {
return this.firstName + " " + this.lastName;
}

public void setDeliveryAddress(String deliveryAddress) {
this.deliveryAddress = deliveryAddress;
}

public String getDeliveryAddress() {
return this.deliveryAddress;
}

}[/CODE]

[CODE lang="java" title="Model: Menu"]package model;

public class Menu {

private String[] menu;
private double[] menuPrices;

public Menu(){
this.setMenu();
this.setMenuPrices();
}

private void setMenu() {
String[] menu = {
"Bunter Salatteller",
"Schnitzel mit Pommes und Salat",
"Angebratener Lachs mit knusprigen Reis und Souce",
"Maultaschen mit Röstzwiebeln und Semmelbrösel",
"Rumpsteak mit Kräuterbutter, Pommes und Salat",
"Veganes Gemüse-Curry mit Kartoffeleinlage und Gemüsevariation",
"Schwäbischer Zwiebelrostbraten mit hausgemachten Spätzle und Salat",
"Schweinelende an Whiskey-Rahmsoße mit hausgemachten Spätzle und Salat"
};

this.menu = menu;
}

public String[] getMenu() {
return this.menu;
}

private void setMenuPrices() {
double[] prices = {
8.00,
11.90,
14.90,
7.90,
19.90,
13.90,
19.90,
16.90
};

this.menuPrices = prices;
}

public double[] getMenuPrices() {
return this.menuPrices;
}
}[/CODE]

[CODE lang="java" title="Model: Order"]package model;

import java.text.DecimalFormat;
import java.util.Arrays;

public class Order {

private String fullName;
private String deliveryAddress;
private int[] selection;
private double price;
private String delivery;
private String deliverTime;

public Order(){

}

public void setFullName(String fullName) {
this.fullName = fullName;
}

public void setDeliveryAddress(String deliveryAddress) {
this.deliveryAddress = deliveryAddress;
}

public void setSelection(int[] selection) {
this.selection = selection;

setPrice();
}

private void setPrice() {
Menu menu = new Menu();
double[] actualPrices = menu.getMenuPrices();
String delivery = getDelivery();
double price = 0;

if (delivery == "Lieferung"){
price = 10;
}

for(int i = 0; i < this.selection.length; i++){
price += actualPrices[this.selection];
}

this.price = price;
}

public void setDelivery(String delivery){
if (delivery.equals("a")) {
delivery = "Abholung";
}else{
delivery = "Lieferung";
}

this.delivery = delivery;
}

public void setDeliverTime(String deliverTime) {
this.deliverTime = deliverTime;
}

public String getFullName() {
return this.fullName;
}

public String getDeliveryAddress() {
return this.deliveryAddress;
}

public String[] getSelection() {
Menu menu = new Menu();
String[] actualMenu = menu.getMenu();
String[] dishes = new String[this.selection.length];

for(int i = 0; i < this.selection.length; i++){
dishes = actualMenu[this.selection];
}

return dishes;
}

public double getPrice() {
return this.price;
}

public String getDelivery() {
return this.delivery;
}

public String getDeliverTime() {
return this.deliverTime;
}

@Override
public String toString(){
DecimalFormat df = new DecimalFormat("##,##0.00 €");
String orderConfirmation;

orderConfirmation = "Das ganze Team bedankt sich bei Ihnen, " + getFullName() + ", für ihre Bestellung. Hier finden Sie ihre Bestellübersicht:\n\n Ihre Gerichte: \n";

for(int i = 0; i < getSelection().length; i++){
orderConfirmation += "- " + getSelection() + "\n";
}

orderConfirmation += "\nStatus: " + getDelivery() + "\nGesamtpreis: " + df.format(getPrice());

if(getDeliveryAddress() != ""){
orderConfirmation += "\nLieferadresse: " + getDeliveryAddress();
}

orderConfirmation += "\nLieferzeit: " + getDeliverTime() + " Uhr";

return orderConfirmation;
}

}[/CODE]

[CODE lang="java" title="Controller: RestaurantPaymentApp"]package controller;

import model.Customer;
import model.Menu;
import model.Order;

import java.text.DecimalFormat;
import java.util.Scanner;

public class RestaurantPaymentApp {

/*
* Projektbeschreibung:
* Diese Anwendung ist ein Bestellsystem über das Kunden über eine Speisekarte ihre Bestellungen tätigen können.
*
* */

public static void main(String[] args) {
Scanner sc = new Scanner(System.in);
Customer customer = new Customer();
Order order = new Order();

welcome();
int action = getAction(sc);
executeAction(action, sc, customer, order);
}

public static void welcome(){
System.out.println("Herzlich Willkommen bei unserem Restaurant 🙂");
System.out.println("Was möchten Sie tun?");

showActions();
}

public static void showActions(){
System.out.println("1. Bestellung tätigen");
System.out.println("2. Beenden");
}

public static int getAction(Scanner sc){
return sc.nextInt();
}

/**
* Zeigt das Menü an
*/
public static void showMenu(){
Menu menu = new Menu();
DecimalFormat df = new DecimalFormat("#0.00 €");

System.out.println("Unser Menü");

for(int i = 0; i < menu.getMenu().length; i++){
System.out.println((i+1) + ") " + menu.getMenu() + ", Preis: " + df.format(menu.getMenuPrices()));
}
}

/**
* Führt die Hauptfunktionen aus
* @param action
* @param sc
* @param customer
* @param order
*/
public static void executeAction(int action, Scanner sc, Customer customer, Order order){

switch (action){
case 1:
System.out.println("Wie ist ihr Vorname?");
customer.setFirstName(capitalize(sc.next()));
System.out.println("Wie ist ihr Nachname?");
customer.setLastName(capitalize(sc.next()));

order.setFullName(customer.getFullName());

trennlinie(50);
showMenu();
trennlinie(50);

System.out.println("Wie viele Gerichte möchten Sie bestellen?");
int amount = sc.nextInt();
int[] selection = new int[amount];
for (int i = 0; i < selection.length; i++){
System.out.println("Welche Gerichte möchten Sie bestellen? (" + (i + 1) + "/" + amount + ") - Bitte geben Sie nur Zahlen ein!");
selection = sc.nextInt() - 1;
}
order.setSelection(selection);
trennlinie(50);

System.out.println("Möchten Sie ihre Bestellung abholen (bitte a eingeben) oder geliefert (bitte g eingeben) bekommen?");
System.out.println("Hinweis: Die Lieferkosten betragen 10€");
String input = sc.next();
order.setDelivery(input);

if(input.equals("a")){
System.out.println("Um wie viel Uhr möchten Sie ihre Bestellung abholen? (Bitte geben Sie nur die Uhrzeit an)");
order.setDeliverTime(sc.next());
}else{
System.out.println();
System.out.println("Um wie viel Uhr möchten Sie ihre Bestellung geliefert bekommen? (Bitte geben Sie nur die Uhrzeit an)");
order.setDeliverTime(sc.next());

System.out.println("An welche Adresse dürfen wir ihre Bestellung liefern?");
sc.nextLine();
customer.setDeliveryAddress(sc.nextLine());
order.setDeliveryAddress(capitalize(customer.getDeliveryAddress()));
}

trennlinie(50);
System.out.println(order);

break;
case 2:
System.out.println("Vielen Dank für Ihren Besuch. Wir wünschen Ihnen einen schönen Tag");

break;
default:
String[] setup = new String[1];
main(setup);
}
}

/**
* Generiert eine Trennline mit der übergebenen Zahl
* @param amount
*
*/
public static void trennlinie(int amount){
String trennlinie = "";

for (int a = 0; a < amount; a++){
trennlinie += "-";
}

System.out.println(trennlinie);
}

/**
* Erster Buchstabe wird zu einem Großbuchstaben umgewandelt
* @param str
* @return
*/

public static String capitalize(String str) {
if(str == null || str.isEmpty()) {
return str;
}

return str.substring(0, 1).toUpperCase() + str.substring(1);
}

}
[/CODE]

Schöne Grüße
 

Robert Zenz

Top Contributor
Java:
package model;

Fuer Test-Aufgaben irrelevant, aber das package sollte die Zugehoerigkeit widerspiegeln, also zum Beispiel "com.github.moonscripter.restaurantpayment" oder "de.moonscripter.restaurantpayment".

Java:
    public void setFirstName(String firstName) {
        this.firstName = firstName;
    }
[CODE]

Notiz am Rande: [URL=https://en.wikipedia.org/wiki/Fluent_interface#Java]Fluent Setters[/URL]
[HR][/HR]
[CODE=java]
    private String[] menu;
    private double[] menuPrices;

Das ist schlecht. Ein Posten auf dem Menue sollte als Objekt dargestellt sein, welches direkt alle notwendigen Informationen buendelt. Auszerdem waere es eventuell interessant eine "List" zu verwenden stattdessen. So das die Sache in etwa so aussieht:

Java:
private List<Item> items = new ArrayList<>();

items.add(new Item("Bunter Salatteller", new BigDecimal("45.30"));
// ...

Damit aendert sich natuerlich die API von "Menu", es liefert nur noch die "Item" Liste aus, nicht mehr die Namen und Preise separat. Hier kannst du direkt auf eine bestimmte Sachen achten, naemlich dass der interne Zustand der Klasse eventuell von aussen veraendert werden kann wenn du das nicht willst.

Java:
public class Menu {
    private List<Item> items = new ArrayList<>();
   
    public Menu() {
        // Init items here.
    }
   
    public List<Item> getItems() {
        return items;
    }
}

Dadurch kann nun jemand von aussen Posten auf dem Menue hinzufugen, da die Liste veraenderbar ist, "menu.getItems().add(new Item())". Das kann man dadurch umgehen dass man entweder nur Kopien von Objekten nach aussen gibt, oder man gibt nicht veraenderbare Varianten raus. In diesem Fall zum Beispiel durch eine nicht veraenderbare Liste:

Java:
public class Menu {
    private List<Item> items = null;
   
    public Menu() {
        List<Item> items = new ArrayList<>();
        // Init items here.
        this.items = Collections.unmodifiableList(items);
    }
   
    public List<Item> getItems() {
        return items;
    }
}

Dadurch kann dann die Liste an Posten von aussen nicht mehr veraendert werden. EIne Anhaenger von "everyhting should be immutable" bin ich im uebrigen nicht, weil es sowohl von einem API-Design Standpunkt, als auch angesichts der Garbage Collection wenig Sinn nicht.

Das gesagt, die Preise sollten BigDecimal sein. Sowohl float als auch double haben Rundungsfehler welcher dir frueher oder spaeter auffallen und Probleme bereiten werden. Vielleicht jetzt nicht in diesem Beispiel, aber in der echten Welt auf jeden Fall. BigDecimal hingegen hat eine arbitraere Genauigkeit.

Java:
public class Order {
    private String fullName;
    private String deliveryAddress;
    private int[] selection;
    private double price;
    private String delivery;
    private String deliverTime;

Die "Order" sollte direkt einen "Customer" halten, vereinfach alles.

Ebenfalls waere es dann hier besser wenn du die "Item"s direkt sammelst, und nicht als Index in das Menue.

Java:
    public void setDelivery(String delivery){
        if (delivery.equals("a")) {
            delivery = "Abholung";
        }else{
            delivery = "Lieferung";
        }

        this.delivery = delivery;
    }

Erstens, loese solches Umsetzen von Werten nicht in deiner Daten/Logik Schicht, sondern dort wo diese notwendig sind, in diesem Fall in deiner UI.

Zweitens, wenn es hier nur zwei Werte geben kann, bietet sich ein Enum an. Vereinfacht auch die Handhabung der Klasse.

Java:
        String trennlinie = "";

        for (int a = 0; a < amount; a++){
            trennlinie += "-";
        }

        System.out.println(trennlinie);

Strings bauen in einer Schleife (wie das hier) sollte immer mit einem StringBuilder erfolgen. Der Hintergrund hier ist dass Strings immutable sind, hier eine kurze Erklaerung:

Java:
String a = readLargeData(); // JVM allocates 500MB for this String.
String b = readLargeData(); // JVM allocates 500MB for this String.

String c = a + b; // Operations are:
// 1. 1000MB (the resulting size of c) is allocated. 2GB are now allocated for all String.
// 2. a is copied into c.
// 3. b is copied into c.

Wie man daran erkennen kann, kann es sehr schnell passieren dass man sehr viel Speicher verbraucht dadurch. Jetzt stell dir vor du fuehrst das in einer Schleife aus:

Java:
String result = "";

for (int counter = 0; counter < 5; counter++) {
    result = result + readLargeData();
}

Der Speicherbedarf pro Durchlauf ist dann wie folgt:
  1. 500MB
  2. 2000MB
  3. 4000MB
  4. 5000MB
  5. 6000MB
Und es muss staendig dieser Speicher kopiert werden. Der StringBuilder haelt sich einen internen Buffer welcher das kopieren auf ein minimum beschraenkt, und damit auch weniger Speicherbedarf hat:

Java:
StringBuilder result = new StringBuilder();

for (int counter = 0; counter < 5; counter++) {
    result.append(readLargeData());
}

result.toString();

Das gilt *nur* fuer solche Schleifen oder sehr grosze Strings (mh, sagen wir 150MB) aufwaerts, das gilt nicht wenn du einfach nur "Tante" und "Emma" auf zwei unterschiedlichen Zeilen zusammen haengen willst.

Dann noch ein paar Tipps am Rande:
  1. Kuerze Namen nicht ab nur weil du kannst, es macht den Code schwieriger zu lesen, zu warten und zu erweitern.
  2. Ich habe die einfache Regel "Man darf Ein-Buchstabige Variablennamen nur bei Dimensioen verwenden", das gilt auch fuer Schleifenzaehler. "index" oder "counter" zu verwenden macht einzelne Zeilen innerhalb der Schleife um ein vielfaches besser lesbar.
  3. Du erzeugst das Menue bei jeder Anzeige neu, das sollte ein globaler Zustand sein.
  4. Du rufst die "main" Funktion rekursiv auf (mit schwachsinnigen Parameter), viel besser waere es wenn du eine Hauptschleife hast welche nur verlassen wird wenn es keine weiteren Bestellungen geben soll.
  5. Verwende einen Code-Formatter, immer. Stell dir das Format so ein wie du es haben willst, aber verwende immer einen Code-Formatter, es macht das Projekt um ein vielfaches leichter zu lesen wenn der Code immer gleich aussieht.
  6. "toString" fuer die Ausgabe auf der Oberflaeche zu verwenden bietet sich an. Aber eine bessere Aufteilung ist wenn sich die UI um das alles kuemmert, und die Datenklassen alle von der UI losgeloest und unabhaengig sind (Uebersetzungen, TUI/GUI und so weiter).
  7. Du haeltst Zeit als String, besser waere es die Benutzereingabe in ein "LocalDateTime/OffsetDateTime" umzuwandeln.
  8. "action" bietet sich ebenfalls als Enum an.
  9. Das gesagt, ein aufteilen der "excuteAction" in einzelne Methoden (so dass das switch nur noch andere Methoden aufruft) wuerde die Lesbarkeit ebenfalls verbessern.
  10. Deine App verwendet im Moment nur statischen Zustand. Vielleicht waere es besser eine eigene UI Klasse zu haben, welche sich um alle Benutzereingaben und das zuordnen zur Logik kuemmert.
  11. Das gesagt, ich bin ein Freund davon eine "Main" Klasse zu haben, welche lediglich die Hauptlogik instanziert und aufruft. Das hat zum einen den Vorteil das man direkt eine "Hauptlogik-Klasse" hat, welche unabhaengig ist, und zum anderen hat dann jemand der neu ist im Projekt immer den Main-Einstiegspunkt den man leicht findet.
 

MoonScripter

Mitglied
Hey Robert,

ich danke dir sehr für deine Zeit und deine ganzen Verbesserungsvorschläge sowie Tipps.

Diese haben mir sehr weitergeholfen :)

Schöne Grüße
 

Neue Themen


Oben