Programm effizienter gestalten

[theqwe]

Hallo zusammen,

ich habe von einem Kollegen ein Code erhalten, mit der Bitte, dass ich das etwas besser anpassen sollte. Leider kann ich ihm da auch nicht viel weiter helfen, weil ich fast genauso umgesetzt hätte. Dafür wollte ich für ihn mal hier nachfragen, ob ihr noch Ideen habt, dass besser umzusetzen.

Er möchte 3 WM Spiele eingeben lassen und sie später als Tabelle ausgeben. Der Gewinner eines Spiels kriegt 3 Punkte und der Verlierer 1 Punkt. Bei Gleichstand kriegen beide 1 Punkt. Wenn am Ende zwei Mannschaften gleich stehen, soll das gesammte Programm wiederholt werden.

Spoiler: Code

import java.util.Scanner;

public class WMgame {

    public static void main(String[] args) {
       
        int punkteDeutschland = 0;
        int punkteItalien = 0;
        int punkteNiederland = 0;
        int toreDeutschland = 0;
        int toreItalien = 0;
        int toreNiederland = 0;
       
        boolean exit = false;

        Scanner tastatur = new Scanner(System.in);
       
        while(exit == false) {
            System.out.println("\n(1) Italien - Deutschland:");
            System.out.print("Tore Italien?: ");
            toreItalien = tastatur.nextInt();
            System.out.print("\nTore Deutschland?: ");
            toreDeutschland = tastatur.nextInt();
           
            if(toreItalien < toreDeutschland) {
                punkteDeutschland += 3;
                punkteItalien += 1;
            }
            else if(toreItalien > toreDeutschland) {
                punkteDeutschland += 1;
                punkteItalien += 3;
            }
            else {
                punkteItalien += 1;
                punkteDeutschland += 1;
            }
            toreItalien = 0;
            toreDeutschland 0;
           
            System.out.println("\n(2) Italien - Niederland:");
            System.out.print("Tore Italien?: ");
            punkteItalien = tastatur.nextInt();
            System.out.print("\nTore Niederlande?: ");
            punkteNiederland = tastatur.nextInt();
           
            if(toreItalien < toreNiederland) {
                punkteDeutschland += 3;
                punkteNiederland += 1;
            }
            else if(toreItalien > toreNiederland) {
                punkteNiederland += 1;
                punkteItalien += 3;
            }
            else {
                punkteItalien += 1;
                punkteNiederland += 1;
            }
            toreItalien = 0;
            toreNiederland = 0;
           
            System.out.println("\n(3) Niederlande - Deutschland:");
            System.out.print("Tore Niederlande?: ");
            punkteNiederland += tastatur.nextInt();
            System.out.print("\nTore Deutschland?: ");
            punkteDeutschland += tastatur.nextInt();
           
            if(toreNiederland < toreDeutschland) {
                punkteDeutschland += 3;
                punkteNiederland += 1;
            }
            else if(toreNiederland > toreDeutschland) {
                punkteNiederland += 1;
                punkteDeutschland += 3;
            }
            else {
                punkteDeutschland += 1;
                punkteNiederland += 1;
            }
            toreDeutschland = 0;
            toreNiederland = 0;
           
            if(punkteDeutschland != punkteItalien && punkteItalien != punkteNiederland && punkteDeutschland != punkteNiederland) {
               
                System.out.println("\n\nMannschaft:\t\tPunkte:");
                System.out.println();
                System.out.println("- Italien\t\t" + punkteItalien);
                System.out.println("- Deutschland\t\t" + punkteDeutschland);
                System.out.println("- Niederland\t\t" + punkteNiederland);
                exit = true;
                tastatur.close();
            }
            else {
                System.out.println("Bei zwei Mannschaften sind die Punkte gleich. Bitte gib neue Punkte ein\n");
            }
        }
    }
}

Was kann man hier verbessern? Es muss doch einen einfacheren Weg geben, anstatt so viele Variablen zu setzen. Mir fällt aber auch nichts ein.

 

[kneitzel]

Also wer verliert bekommt 1 Punkt? Interessante Regelung.

Was kennt ihr denn so?

a) Klassen? Sowas könnte man dann doch objektorientiert machen. Es gibt Mannschaften, die haben einen Namen und einen Punktestand. Dann gibt es die Tabelle. Der gehören Mannschaften an und da kann man dann Spielergebnisse hinzufüǵen und eine Tabelle ausgeben ....

b) Noch keine Klassen? Was für Datenstrukturen kennt ihr denn da? Maps? Dann kann man Namen zu Punkten zuordnen.... Oder wenn man einfach nur Arrays verwendet ....

 

[MoxxiManagarm]

Der Code zeigt sehr viel Code-Duplikate. Für eine Codeverbesserung würde ich dort ansetzen das zu reduzieren. Dann kommst du eigentlich um bestimmte Datenstrukturen siehe Post #2 von JustNobody nicht herum.

Gehen wir von Klassen aus. Dann könntest du für einen Durchlauf etwa sowas schreiben:

// Mannschaften
Team de = new Team("Deutschland");
Team nl = new Team("Niederlande");
Team it = new Team("Italien");

// Begegnungen
League.match(de, nl);
League.match(nl, it);
League.match(de, it);

// Auswertungen
League.evaluate(de, nl, it);

 

[theqwe]

Also wer verliert bekommt 1 Punkt? Interessante Regelung.

Was kennt ihr denn so?

Habe ich auch nicht verstanden, er sagte aber, ist eine Vorgabe
Ich selbst hätte es natürlich auch objektorientiert gemacht, aber soweit ist er noch nicht. Deshalb weiß ich auch nicht, wie ich ihm das anders zeigen soll. Die machen scheinbar im Moment nur Klassen.

 

[mihe7]

Mal ein Vorschlag, der ohne Methoden & Co. auskommt. Wenn ich es richtig sehe, dann bekommt jede Mannschaft für ein Spiel in jedem Fall mal einen Punkt. Der Gewinner erhält nochmal 2 Punkte oben drauf.

Das führt schon mal zu einer gewissen Verkürzung:

            punkteItalien += 1;
            punkteDeutschland += 1;
            if(toreItalien < toreDeutschland) {
                punkteDeutschland += 2;
            }
            else if(toreItalien > toreDeutschland) {
                punkteItalien += 2;
            }

Mit dem ternären Operator kann man zuerst die ifs killen:

            punkteItalien += 1;
            punkteDeutschland += 1;
            punkteDeutschland += toreItalien < toreDeutschland ? 2 : 0;
            punkteItalien += toreItalien > toreDeutschland ? 2 : 0;

und dann zusammenfassen:

            punkteItalien += 1 + (toreItalien > toreDeutschland ? 2 : 0);
            punkteDeutschland += 1 + (toreItalien < toreDeutschland ? 2 : 0);

 

[theqwe]

Sehr guter Vorschlag. Da lerne sogar ich noch etwas dazu. Das mit den ternären Operatoren kenne ich nämlich auch noch nicht.
Ich gebe es mal so weiter. Das verkürzt es echt um einiges.
Vielen Dank von mir und ihm schonmal.

 

[MoxxiManagarm]

Ich selbst hätte es natürlich auch objektorientiert gemacht, aber soweit ist er noch nicht. Deshalb weiß ich auch nicht, wie ich ihm das anders zeigen soll. Die machen scheinbar im Moment nur Klassen.

Verstehe ich nicht, was heißt für dich nur Klassen? Wären Klassen wie "League", "Match" und "Team" nicht auch nur Klassen? Trotzdem ist das doch objektorientiert?!

 

[Blender3D]

ich habe von einem Kollegen ein Code erhalten, mit der Bitte, dass ich das etwas besser anpassen sollte. Leider kann ich ihm da auch nicht viel weiter helfen, weil ich fast genauso umgesetzt hätte. Dafür wollte ich für ihn mal hier nachfragen, ob ihr noch Ideen habt, dass besser umzusetzen.

Mit Arrays und Funktionen lässt sich der Code durchaus verbessern.

package wmtable;

import java.util.Scanner;

public class WmGame {
    private static final Scanner in = new Scanner(System.in);
    private static final String[] team = { "Deutschland", "Italien", "Niederlande" };
    private static final int GER = 0;
    private static final int ITA = 1;
    private static final int NED = 2;

    public static void main(String[] args) {
        int[] points = new int[team.length];

        boolean exit = false;
        while (exit == false) {
            inputMatch(points, ITA, GER, 1);
            inputMatch(points, ITA, NED, 2);
            inputMatch(points, NED, GER, 3);

            if (!isOneOrMoreStandOff(points)) {
                printTable(points);
                exit = true;
            } else {
                reset(points);
                System.out.println("Bei zwei Mannschaften sind die Punkte gleich. Bitte gib neue Punkte ein\n");
            }
        }
        in.close();
    }

    private static void evaluateResult(int[] points, int teamA, int teamB, int[] goals) {
        if (goals[teamA] == goals[teamB]) {
            points[teamA] += 1;
            points[teamB] += 1;
            return;
        }
        if (goals[teamA] > goals[teamB]) {
            points[teamA] += 3;
            points[teamB] += 1;
        } else {
            points[teamA] += 3;
            points[teamB] += 1;
        }
    }

    private static int inputInt(String label) {
        int value = 0;
        boolean ok = false;
        do {
            try {
                System.out.print(label);
                value = Integer.parseInt(in.nextLine());
                ok = true;
            } catch (NumberFormatException e) {
                System.err.println("Bitte nur ganze Zahlen eingeben!");
            }
        } while (!ok);
        return value;
    }

    private static void inputMatch(int[] points, int teamA, int teamB, int num) {
        System.out.println("(" + num + ")\t" + team[teamA] + " - " + team[teamB]);
        int[] goals = new int[team.length];
        goals[teamA] = inputInt("Tore " + team[teamA] + " : ");
        goals[teamB] = inputInt("Tore " + team[teamB] + " : ");
        evaluateResult(points, teamA, teamB, goals);

    }

    private static boolean isOneOrMoreStandOff(int[] points) {
        for (int i = 0; i < points.length - 1; i++) {
            for (int ii = i + 1; ii < points.length; ii++) {
                if (points[i] == points[ii])
                    return true;
            }
        }
        return false;
    }

    private static void printTable(int[] points) {
        System.out.println("\n\nMannschaft:\t\tPunkte:\n");
        for (int i = 0; i < team.length; i++)
            System.out.println("- " + team[i] + "\t\t" + points[i]);
    }

    private static void reset(int[] data) {
        for (int i = 0; i < data.length; i++)
            data[i] = 0;
    }

}

 

[theqwe]

Hallo zusammen,

ich wollte das Thema nochmal aufgreifen. Wollte mal nach meiner Art der objektorientierten Programmierung seinen Code mal umschreiben. Das Problem ist, dass ich keinen guten Weg finde, auf die "Spiel-Methode" zu zugreifen. Mit einer Instanzvariable bringt das wohl nur einen NullPointer.

package klausur10;

import java.util.Scanner;

public class WMSpiel {

    public static void main(String[] args) {

        Scanner tastatur = new Scanner(System.in);
        Mannschaft team1 = new Mannschaft("Italien", 0, 0);
        Mannschaft team2 = new Mannschaft("Deutschland", 0, 0);
        Mannschaft team3 = new Mannschaft("Niederlande", 0, 0);
        Mannschaft spiel = null;
        boolean run = true;
        
        while(run) {
            System.out.print("1. Spiel: Deutschland - Italien");
            System.out.print("\nTore Deutschland: ");
            team2.tore = tastatur.nextInt();
            System.out.print("\nTore Italien: ");
            team1.tore = tastatur.nextInt();
            
            System.out.print("\n\n2. Spiel: Niederlande - Italien");
            System.out.print("\nTore Niederlande: ");
            team3.tore = tastatur.nextInt();
            System.out.print("\nTore Italien: ");
            team1.tore = tastatur.nextInt();
            
            System.out.print("\n\n2. Spiel: Niederlande - Deutschland");
            System.out.print("\nTore Niederlande: ");
            team3.tore = tastatur.nextInt();
            System.out.print("\nTore Deutschland: ");
            team2.tore = tastatur.nextInt();
            
            spiel.Spiel(team2, team1, team2.tore, team1.tore);
            
            if(team1.punkte != team2.punkte && team1.punkte != team3.punkte && team2.punkte != team3.punkte) {
                
                System.out.println("Mannschaft\t\tPunkte\n");
                System.out.println(team1);
                System.out.println(team2);
                System.out.println(team3);
                run = false;
                tastatur.close();
            }
            
            else {
                
                System.out.print("Zwei Mannschaften haben die gleichen Punkte.\nBitte eingabe wiederholen!\n\n");
            }
        }
    
    }
}

package klausur10;

public class Mannschaft {
    
    String name;
    int punkte;
    int tore;
    
    public Mannschaft(String name, int punkte, int tore) {
        
        this.name = name;
        this.punkte = punkte;
        this.tore = tore;
    }

    
    public void Spiel(Mannschaft erstesTeam, Mannschaft zweitesTeam, int toreErstesTeam, int toreZweitesTeam) {
        
        if(toreErstesTeam < toreZweitesTeam) {
            
            erstesTeam.punkte += 1;
            zweitesTeam.punkte += 3;
        }
        
        else if(toreErstesTeam > toreZweitesTeam) {
            
            erstesTeam.punkte += 3;
            zweitesTeam.punkte += 1;
        }
        
        else {
            
            erstesTeam.punkte += 1;
            zweitesTeam.punkte += 1;
        }
        
        erstesTeam.tore = 0;
        zweitesTeam.tore = 0;
    }
    
    public String toString() {
        
        return name + "\t" + punkte;
    }
}

 

[kneitzel]

Du hast ja spiel auch nie initialisiert. Aber das macht ja so auch wenig Sinn.

Also wenn Du eine Methode schreibst, dann nutzt diese in der Regel die Instanz, auf der diese aufgerufen wird. (Also "this" in der Methode!)
Wenn Du das nicht verwendest, dann kannst Du die Methode direkt static machen. ==> Dann würde die Methode ohne Instanz aufgerufen als Mannschaft.spiel(...)
a) Methoden werden klein geschrieben. Wäre super, wenn Du das auch machen könntest.
b) Das funktioniert aber ist ein ganz mieses Design!

Also solltest du das richtig modellieren. Mannschaften spielen gegeneinander. Also hast Du eine Methode wie:

spielt(final Mannschaft gegner, final int eigeneTore, final int gegnerischeTore)

In der Methode hast die eigene Mannschaft in "this" - und kannst dann also Deine Methode einfach verwenden und musst nur ein paar Dinge umbenennen:
erstesTeam -> this
zweitesTeam -> gegner
toreErstesTeam -> gegnerischeTore
toreZweitesTeam -> eigeneTore

Und der Aufruf ändert sich dann halt auch, denn das erste Team ist dann nicht mehr Parameter sondern Du rufst die Methode darauf dann auf.

 

[mihe7]

Jetzt hat mir @JustNobody schon mal eine ganze Menge abgenommen

Bleibt mir nur noch die Anmerkung, dass die Methode in der Klasse Mannschaft nicht besonders gut aufgehoben ist: zwar spielen Mannschaften gegeneinander, vergeben aber weder sich selbst noch anderen Mannschaften Punkte.

 

[theqwe]

Und der Aufruf ändert sich dann halt auch, denn das erste Team ist dann nicht mehr Parameter sondern Du rufst die Methode darauf dann auf.

Sowas hatten wir auch mal in der Vorlesung. Meinst du damit sowas wie team1.spielt
(final Mannschaft gegner, final int eigeneTore, final int gegnerischeTore) ?

vergeben aber weder sich selbst noch anderen Mannschaften Punkte.

Das muss ich mal im Debugger angucken. Werden damit die Punkte nicht in die Objektvariablen gesetzt?

 

[kneitzel]

Das war die neue Signatur. Falls das final dich irritiert hat, dann löschen es nur raus.

Wenn du die Signatur wie angegeben (egal ob mit oder ohne die final) geändert hast, dann kannst du den Aufruf über z.B. team1 machen. Aber beim Aufruf musst du natürlich nur Parameter angeben, also z.B. team1.spielt(team2, 3, 1);

Damit berücksichtigt du aber noch nicht den Hinweis von mihe7. Aber es ist ein erster Anfang...

 

[theqwe]

Falls das final dich irritiert hat, dann löschen es nur raus.

Das gibt doch nur an, dass die Werte "feste" sind und nicht mehr veränderbar, korrekt? Hab versehentlich einfach alles kopiert. Meinte natürlich teamx.spielt(gegner, tore, toregegner) jetzt variable kurz gemacht.

Das von mihe7 verstehe ich noch nicht ganz. Werden die Werte nicht gespeichert?

 

[mihe7]

Das von mihe7 verstehe ich noch nicht ganz. Werden die Werte nicht gespeichert?

Doch. Mir ging es darum, dass durch die Entscheidung, die Methode in die Klasse Mannschaft aufzunehmen, eine Mannschaft die Verantwortung bekommt, die Punkte an sich und den Gegner zu verteilen.

 

[theqwe]

Achso. Klingt jetzt als wäre das keine gute Idee. Soll die Methode also am besten in WMSpiel?

 

[mihe7]

Das kommt darauf an, wie weit man gehen, d. h. was man genau abbilden will. Wenn Du Dir Kommentar #3 von @MoxxiManagarm anschaust, siehst Du einen deutlichen Unterschied: dort sind Mannschaften einfach Mannschaften und es gibt eine zusätzliche Klasse (League), die die Begegnungen und deren Ergebnisse (inkl. Punkte) verwaltet.

Der Kernpunkt ist, dass die Spielregeln festlegen, wer bei welchem Ergebnis wie viele Punkte erhält und es "jemanden" (=Objekt) geben sollte, der die Spielregeln durchsetzt.

 

[MoxxiManagarm]

Ich meinte das ungefähr so:

package de.javaforum.wm;

import java.util.ArrayList;
import java.util.List;

public class WM {
    List<Team> teams = new ArrayList<>();
    List<Match> matches = new ArrayList<>();

    public WM(String... teamNames) {
        for (String teamName : teamNames) {
            this.teams.add(new Team(teamName));
        }
    }

    public void registerMatches() {
        // quick & dirty, will ich hier jetzt nicht ausprogrammieren, geht aber theoretisch einfach für jede beliebige Anzahl Teams
        matches.add(new Match(teams.get(0), teams.get(1), 1));
        matches.add(new Match(teams.get(1), teams.get(2), 2));
        matches.add(new Match(teams.get(0), teams.get(2), 3));
    }

    public void start() {
        for (Match match : matches) {
            match.play();
            match.score();
        }
    }

    public void printResults() {
        System.out.println("Matches played:");
        for (Match match : matches) {
            System.out.println(match);
        }

        System.out.println("Scorings:");
        for (Team team : teams) {
            System.out.println(team);
        }
    }

    public static void main(String... args) {
        WM wm = new WM("Germany", "Netherlands", "Italy");

        wm.registerMatches();
        wm.start();
        wm.printResults();
    }
}

package de.javaforum.wm;

import java.util.Random;

public class Match {
    private Team team1;
    private Team team2;
    private int matchday;
    private int goalsTeam1;
    private int goalsTeam2;

    public Match(Team team1, Team team2, int matchday) {
        this.team1 = team1;
        this.team2 = team2;
        this.matchday = matchday;
    }

    public void play() {
        Random random = new Random();

        goalsTeam1 = random.nextInt(6);
        goalsTeam2 = random.nextInt(6);
    }

    public void score() {
        if (goalsTeam1 > goalsTeam2) {
            team1.addPoints(3);
            team2.addPoints(1);
        } else if (goalsTeam1 < goalsTeam2) {
            team1.addPoints(1);
            team2.addPoints(3);
        } else {
            team1.addPoints(1);
            team2.addPoints(1);
        }
    }

    @Override
    public String toString() {
        return String.format("[%d] %s %d : %d %s", matchday, team1.getName(), goalsTeam1, goalsTeam2, team2.getName());
    }
}

package de.javaforum.wm;

public class Team {
    private String name;
    private int points;

    public Team (String name) {
        this.name = name;
        this.points = 0;
    }

    public String getName() {
        return name;
    }

    public void addPoints(int points) {
        this.points += points;
    }

    @Override
    public String toString() {
        return String.format("%d - %s", points, name);
    }
}

Das setzt natürlich nicht alles um was du brauchst (z.B. nicht die Wiederholung), aber im Großen und Ganzen ist es das schon