Lotterie in der Kommandozeile

[Humbolzen]

Ich habe in Java ein Programm geschrieben, in dem der Benutzer Lottozahlen erraten soll.
Ich wäre dankbar, wenn ein waschechter Programmierer meinen Code hinsichtlich folgender Aspekte bewerten kann:

1. Genügt dieses Programm den Ansprüchen der objektorientierten Programmierung?
2. Wurden die Mittel der Sprache Java ausgenutzt, oder wurden Teile des Programms umständlicher realisiert, als sie sein müssen?
3. Stehen Ein- und Ausgabeanweisungen an der richtigen Stelle?
4. Gibt es außerhalb dieser Kriterien noch andere Dinge, die man am Code verbessern kann?

Ich wäre dankbar für eine Bewertung!

Das Programm kann hier getestet werden:
https://repl.it/@Dexter1997/FullEducatedToolbox

Main.java

public class Main {
    public static void main(String[] args) {
            Game game = new Game();
            game.play();
    }
}

Game.java

import java.util.Collections;
import java.util.List;
import java.util.ArrayList;
import java.util.Scanner;

public class Game {
    private Lottery lottery;
    private List<Integer> guess;
    private Integer guessedNumber;
    private boolean inputAccepted;
    private int rightNumbers;
    private static Scanner scanner = new Scanner(System.in);
   
    public Game() {
        lottery = new Lottery();
        guess = new ArrayList<Integer>();
        guessedNumber = 0;
        inputAccepted = false;
        rightNumbers = 0;
    }
   
    public void play() {
        System.out.println("Es gibt eine neue Ziehung!");
        System.out.println("Sie müssen nun sechs Zahlen erraten.\n");
        guessNumbers();
        compareGuess();
        printScore();
    }
   
    private void guessNumbers() {
        while (guess.size() < 6) {
            System.out.print("Zahl Nr." + (guess.size() + 1) + ": ");
            String input = scanner.nextLine();
            guessedNumber = Integer.valueOf(input);
            checkInput();
            if (inputAccepted) {
                guess.add(guessedNumber);
            }
        }
        System.out.println();
        Collections.sort(guess);
    }
   
    // checks if input is valid
    private void checkInput() {
        inputAccepted = true;
        // check if number is out of range
        if (guessedNumber > 49 || guessedNumber < 1) {
            System.out.println("Bitte eine Zahl zwischen 1 und 49 eingeben.");
            inputAccepted = false;
        }
       
        // check if number allready exists in list
        for (int i = 0; i < guess.size(); i++) {
            if (guess.get(i).equals(guessedNumber)) {
                System.out.println("Diese Zahl haben sie bereits getippt.");
                inputAccepted = false;
                break;
            }
        }
    }
   
    private void compareGuess() {
        for (int i = 0; i < 6; i++) {
            for (int j = 0; j < 6; j++) {
                if (lottery.getDraw().get(i).equals(guess.get(j))) {
                    rightNumbers++;
                }
            }
        }
    }
   
    private void printScore() {
        System.out.println("Lottozahlen: " + lottery.getDraw());
        System.out.println("Dein Los:    " + guess);
       
        if (rightNumbers == 0) {
            System.out.println("Sie haben keine einzige Zahl erraten.");
        } else if (rightNumbers == 1) {
            System.out.println("Sie haben eine Zahl richtig erraten.");
        } else if (rightNumbers > 1) {
            System.out.println("Sie haben " + rightNumbers + " Richtige.");
        }
    }
}

Lottery.java

import java.util.Collections;
import java.util.List;
import java.util.ArrayList;
import java.util.Random;

public class Lottery {   
    private static Random random = new Random();
    private List<Integer> draw;
   
    public Lottery() {
        renewDraw();
    }
   
    public List<Integer> getDraw() {
        return draw;
    }
   
    public void renewDraw() {
        draw = new ArrayList<Integer>();
        while (draw.size() < 6) {
            Integer number = new Integer(random.nextInt(49) + 1);
            boolean drawHasNumber = false;
            for (int i = 0; i < draw.size(); i++) {
                if (draw.get(i).equals(number)) {
                    drawHasNumber = true;
                    break;
                }
            }
            if (!drawHasNumber) {
                draw.add(number);
            }
        }
        Collections.sort(draw);
    }
   
    public int compare(List<Integer> guess) {
        int rightNumbers = 0;
         for (int i = 0; i < 6; i++) {
             for (int j = 0; j < 6; j++) {
                 if (draw.get(i).equals(guess.get(j))) {
                     rightNumbers++;
                 }
             }
         }
         return rightNumbers;
    }
}

 

[rob-]

Was mir direkt auffällt sind die doppelten for Schleifen. Keine Java 8 Streams möglich?

Du sortierst auch beide Zahlenmengen, vergleichst aber dennoch alle Elemente. Könntest abbrechen, sobald eine Zahl in Lottery größer der eingegebenen Zahl. Falls du nur die richtigen Treffer ausgeben willst, könntest auch die Listen mergen und nur Duplikate mitnehmen.

 

[Abcedy]

Insgesamt schaut's schon mal OK aus.

Wann immer möglich, sollte void vermieden werden.
Denn Funktionen mit return sind wesentlich besser zu kontrollieren.

Weitere Optimieren sind u.a. dass compare(2xfor) durch contains zu kürzen&beschleunigen.
Statt for() wird heutzutage foreach() verwendet, dies erlaubt kürzere Schreibweisen und Zugriff auf den Key und das Element.

 

[DerDev256]

Mir fallen gerade 3 Sachen auf, wenn ich den Code so überfliege.

1. In der Game Klasse würde ich für guess ein Array statt eine ArrayList verwenden (wenn die Länge der "Liste" schon beim Code schreiben bekannt ist und sich nicht ändert >> immer ein Array verwenden) [als mögliche Optimierung]
2. In der Game Klasse gehst du von einem "schlauen" Anwender aus: Wenn er sein Tipp abgibt, gehst du davon aus, dass er eine Zahl eingibt. Wenn er jedoch ein Trigger Bot ist und einen Buchstaben o.ä. eingibt, dann stürzt dann Programm ab. Dazu gebe es zwei Lösungen:
   1. try-catch mit NumberFormatException um Integer.valueOf() und Eingabe wiederholen lassen.
   2. Scanner.nextInt() benutzen und try-catch mit InputMismatchException und Eingabe wiederholen lassen.
3. In der Game Klasse mit printScore() switch-case statt if-else verwenden.

Ansonsten schließe ich mich meinen Vorrednern an, sieht schon ganz solide aus

LG,
DeepMeep

 

[GeneralMelchett]

Da du hier effektiv mit Mengen arbeitest sind Sets eigentlich deutlich besser geeigent als Lists. Insbesondere mit den Methoden contains(...) und retainAll(...) kannst du dir viel Arbeit sparen. Außerdem:
- Game.checkInput() sollte sein Ergebnis als Rückgabewert und nicht als Feld übermitteln. Das ist sonst nur unnötig verwirrend.
- Benutzt du Lottery.compare(...) überhaupt und macht es nicht letztlich das gleiche wie Game.compareGuess()?

 

[Humbolzen]

Echolalie​

Ups, da hast du Recht, ich hätte ganz vergessen dass ich die Methode compare() habe...