2017-03-05 3 views
1

Ich habe dieses Skript, um 2 Würfelrollen in Java zu simulieren. Dies geschieht zweimal, einmal vom Benutzer und einmal vom Computer (beide automatisch). Das Programm gibt die Walzen aus und summiert sie. Ich kann jedoch keine if/else-Anweisung erhalten, um die Rollen zu vergleichen und den Gewinner/die Krawatte zu bestimmen. Bisher habe ich:Vergleichen von Int in Java

import java.util.Scanner; 
import java.util.Random; 
import java.text.DecimalFormat; 
/* 
Program to simulate die roll 
*/ 
public class Dice 
{ 
Scanner scan = new Scanner (System.in); 
Random generator= new Random(); 
int roll1; 
int roll2; 
int roll3; 
int roll4; 
int addroll1; 
int addroll2; 

public void userdieroll() // Simulates users role 
{ 
    roll1 = (generator.nextInt(7) +1); // Generate number from 1-6 
    System.out.println("Your first roll is "+roll1+"");// Says users first role 
    roll2 = (generator.nextInt(7) +1); // Generate number from 1-6 
    System.out.println("Your second roll is "+roll2+"");// Says users second roll 
    addroll1= roll1 +roll2;// Sums users roles 
    System.out.println("The sum of your two roles is "+addroll1+" \n"); 
} 
public void compdieroll()// Simulates computers role 
{ 
    roll3 = (generator.nextInt(7) +1); // Generate number from 1-6 
    System.out.println("The computers first role is "+roll3+""); // Says computers first role 
    roll4 = (generator.nextInt(7) +1); // Generate number from 1-6 
    System.out.println("The computers second role is "+roll4+""); // Says computers second role 
    addroll2= roll3 +roll4;// Sums computers roles 
    System.out.println("The sum of the computers roles is "+addroll2+""); 
} 
public void findwinner() 
{ 
     if (addroll1 == addroll2) 
     { 
      System.out.println("Its a tie!"); 
     } 
     else 
     { 
      if (addroll1 > addroll2) 
      { 
       System.out.println("You Won!"); 
      } 

      else 
      { 
       System.out.println("You lost!"); 
      } 

} 
} 
public static void main(String[] args) 
{ 
    Dice userroll = new Dice(); 
    userroll.userdieroll(); 
    Dice comproll = new Dice(); 
    comproll.compdieroll(); 
    Dice looper = new Dice(); 
    looper.findwinner(); 

} 

} 
+0

'roll1 = (generator.nextInt (7) +1); // Generiere Nummer von 1-6, um, nein. Generiere eine Zahl von 0 bis 6 und füge dann eine hinzu. Du erhältst die Zahlen 1 bis 7 (inklusive). –

+0

Ja oops mein schlechter, schöner Fang – user2860253

Antwort

2

Sie eine neue Dice Objekt jedes Mal erstellen Sie eine Methode aufrufen. Wenn Sie dies tun, speichern Sie nicht addroll1 und addroll2 in demselben Objekt, so ist es nur natürlich, dass .findwinner() nicht wie erwartet in Ihrem dritten Objekt funktioniert, wo Sie keine Werte in addroll1 und addroll2 gespeichert haben. Um dies zu beheben, verwenden Sie das gleiche Dice Objekt für alle drei Methoden, etwa so:

Dice tester = new Dice(); 
tester.userdieroll(); 
tester.compdieroll(); 
tester.findwinner(); 
+0

Danke! Das war mir nicht bewusst! Es macht Sinn, wenn Sie darüber nachdenken! – user2860253

+0

Gern geschehen! – UnknowableIneffable

1

Mmmh, von einem ersten Blick Sie Instanziieren drei verschiedene Objekte. Und findwinner() arbeitet an addroll1 = 0 und addroll2 = 0 seit 0 ist der Standardwert für diese Klasseneigenschaft. Wenn Sie an denselben Daten von verschiedenen Objekten arbeiten möchten, müssen Sie die Eigenschaften static eingeben. Wie auch immer vorgeschlagen, tun Sie den Job in einer einzigen Instanz.

+0

Die Eigenschaften 'statisch' zu machen ist ein schrecklicher Ansatz. Man sollte keinen veränderbaren statischen Zustand halten; Das gibt Ihnen den ganzen Schmerz globaler Variablen, wie fehlende Testbarkeit oder Skalierbarkeit und Verlust der Kontrolle über den Objektlebenszyklus. @UnknownableInffible [sic] hat die bessere Lösung. –

+0

Ich weiß. Wie gesagt, der beste Ansatz besteht darin, eine einzelne Instanz mit ihren Eigenschaften zu verwenden. Aber das OP verwendet drei verschiedene Objekte, so dass statische ist durchaus akzeptabel. – LppEdd

+0

Ich schlage vor, dass die Antwort, die nicht auf die antipattern "statisch veränderlichen Zustand" suggeriert, akzeptabel ist, nicht diejenigen, die diese antipattern vorschlagen. –

0

Verwenden Sie nur ein Objekt, das Sie erstellen 3 Würfel und diese Werte sind nicht statisch zwischen Instanzen gemeinsam genutzt ...

Dies funktioniert:

Dice userroll = new Dice(); 
userroll.userdieroll(); 
userroll.compdieroll(); 
userroll.findwinner(); 
-1

In Ihrer Klasse beide gibt es die Benutzer Würfel als die Computerwürfelvariablen. in der Haupt sollten Sie nur eine Klasse erstellen:

Dice d = new Dice(); 
d.userdieroll(); 
d.compdieroll(); 
d.findwinner(); 

Ihr Code erstellt drei verschiedene Instanzen von Würfeln mit drei roll1 verschiedenen, roll2, ... wenn Sie

... 
Dice looper = new Dice(); 
looper.findwinner(); 

schreiben vergleicht es varlable looper.addroll1 mit looper.addroll2 (dh diejenigen innerhalb der Looper-Klasse), aber diese Variablen sind immer noch 0 (Standardwert), in den Zeilen vor dem Auffüllen userroll.addroll1 und comproll.addroll2.

Vjncenzo

+0

Es ist falsch, dass der Code des OP "drei verschiedene Klassen erzeugt" und auch falsch, dass die Variablen "addroll1" nicht initialisiert wurden ". –

+0

Sie haben Recht, ich habe die Antwort korrigiert. Danke – Vjncenzo

1

Nur um Ihnen eine Vorstellung zu geben, wie ich es umgesetzt haben würde (mit über 20 Jahren Erfahrung der Programmierung mit Java):

package nl.owlstead.stackoverflow; 

import java.util.Random; 

/* 
* Program to simulate a double die roll. 
*/ 
public class DiceGame { 
    private Random generator = new Random(); 

    enum Result { 
     LOSER, 
     TIE, 
     WINNER; 
    } 

    public DiceGame() { 
     // nothing to do in constructor 
    } 

    public int rollDice() { 
     int zeroToFive = generator.nextInt(6); 
     int oneToSix = zeroToFive + 1; 
     return oneToSix; 
    } 

    public Result findWinner(int totalRollComp, int totalRollUser) { 
     if (totalRollComp > totalRollUser) { 
      return Result.LOSER; 
     } 

     if (totalRollComp < totalRollUser) { 
      return Result.WINNER; 
     } 

     return Result.TIE; 
    } 

    public static void main(String[] args) { 
     DiceGame game = new DiceGame(); 
     int firstRollComp = game.rollDice(); 
     int secondRollComp = game.rollDice(); 
     int totalRollComp = firstRollComp + secondRollComp; 
     System.out.printf("Comp rolled: %d and %d, totalling %d%n", firstRollComp, secondRollComp, 
       totalRollComp); 

     int firstRollUser = game.rollDice(); 
     int secondRollUser = game.rollDice(); 
     int totalRollUser = firstRollUser + secondRollUser; 
     System.out.printf("User rolled: %d and %d, totalling %d%n", firstRollUser, secondRollUser, 
       totalRollUser); 

     Result userResult = game.findWinner(totalRollComp, totalRollUser); 
     switch (userResult) { 
     case LOSER: 
      System.out.println("You lost!"); 
      break; 
     case TIE: 
      System.out.println("Its a tie!"); 
      break; 
     case WINNER: 
      System.out.println("You Won!"); 
      break; 
     } 
    } 
} 

Was besonders nützlich ist, ist wie einige Felder zu verwenden, wie möglich. Wie Sie bereits herausgefunden haben mit Feldern (was Zustand in einem Programm darstellt) ist ziemlich gefährlich. Wenn Sie Status vermeiden können, sollten Sie dies tun. Also in diesem Programm gibt es nur ein Feld: den Zufallszahlengenerator (natürlich würde ich stattdessen new SecureRandom() verwenden).

Darüber hinaus zeigt es auch die Stärke der Auswahl guter Namen für Ihre Variablen und Splitting-Ausgabe (System.out) und das Spiel selbst. Ich habe versucht, nicht zu viele Techniken neben dem einzuführen, aber ich konnte nicht widerstehen, eine Enum für das Ergebnis und printf zu verwenden, um die verschiedenen println Aussagen zu vereinfachen.

+0

Ausgezeichnete Antwort, die mehrere Best Practices veranschaulicht. In einem Konstruktor nichts außer der Initialisierung zu tun, wird viel zu häufig verletzt. –