Korrekter Quelltext?

[ehlr]

Hallo.
Ich habe eine Forms geschrieben, die ein Quadrat mit einer Zahl (Würfel) anzeigt.
Klickt man auf den Button, so würfelt das Programm eine neue Zahl und wechselt per Zufall die Farbe des Würfels.

Ist mein Quelltext korrekt geschrieben, oder gibt es vermeidbare Fehler ? Kann man etwas anders oder besser machen?

Starter.java

public class Starter {
	
	public static void main(String[] args) {
		new Würfel();
	}
}

Würfel.java

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class Würfel extends JFrame {

	/*
	 * Instanzvariablen
	 */
	
	private JButton button;
	private JPanel panel;
	private JLabel label;
	private Quadrat quadrat = new Quadrat();
	
	/*
	 * Konstruktor
	 */
	
	public Würfel() {
		super("Cube");
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        init();
        pack();
        setLocationRelativeTo(null);
        setResizable(false);
        setSize(100,170);
        quadrat.setPreferredSize(new Dimension(100, 100));
        setVisible(true);
	}
	
	/*
	 * Methode
	 */
	
	private void init() {
		panel = new JPanel();
		button = new JButton("Würfeln");
		label = new JLabel("0");
		label.setFont(new Font("Arial", Font.BOLD, 75));
		label.setForeground(Color.WHITE);
		quadrat.add(label);
		panel.add(quadrat);
		panel.add(button);
		getContentPane().add(panel);
		
		button.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent event) {
	            panel.repaint();
	            int zahl = quadrat.zufall();
	            label.setText(zahl +"");
	        }
	    });
	}   
}

Quadrat.java

@SuppressWarnings("serial")
public class Quadrat extends JPanel{
 
	/*
	 * Methoden
	 */
	
      @Override
  public void paintComponent( Graphics g )
  {      
    int x1 = (int) (Math.random() * 150);
    int x2 = (int) (Math.random() * 150);
    int x3 = (int) (Math.random() * 150);
    g.setColor(new Color(x1,x2,x3));
    g.drawRect(0,0,100,100);
    g.fillRect(0,0,100,100);
  }
      
  public int zufall() {
	  int zahl;
	  zahl = (int) (Math.random() * 6) + 1;
	  return zahl;
  }
}

lg
ehlr

 

[Fab1]

Ich glaube es wurde mal geraten, dass in der paintComponent Methode nur gezeichnet werden soll. Und sonst nichts anderes. Aber ansonsten sieht es gut aus.

 

[gman]

Hi,

Umlaute in Klassen und Variablen sollte man auch eher vermeiden.

 

[xehpuk]

[JAVA=4]new Würfel();[/code]
Mit Swing-Komponenten sollte man immer auf dem Event Dispatch Thread hantieren. Dafür verwendet man meistens

SwingUtilities.invokeLater()

.
__________

[JAVA=32]pack();
setLocationRelativeTo(null);
setResizable(false);
setSize(100,170);[/code]
Ist das wirklich so gewollt? Nicht eher so (richtig zentriert)?
[JAVA=32]setResizable(false);
setSize(100, 170);
setLocationRelativeTo(null);[/code]
__________

[JAVA=57]panel.repaint();[/code]
Das ergibt keinen Sinn. Wozu neuzeichnen, wenn nichts passiert ist?
__________

[JAVA=11]int x1 = (int) (Math.random() * 150);
int x2 = (int) (Math.random() * 150);
int x3 = (int) (Math.random() * 150);
g.setColor(new Color(x1,x2,x3));[/code]
Wie bereits erwähnt, hat das in einer paint-Methode nichts zu suchen. Du musst eine Stelle suchen, in der du das Erstellen der zufälligen Farbe erledigst. Anbieten würde sich wohl die

actionPerformed()

.
Des Weiteren fehlt der Aufruf von

super.paintComponent()

. Deswegen wird vorherig Gezeichnetes nicht entfernt.
__________

[JAVA=19]public int zufall() {
int zahl;
zahl = (int) (Math.random() * 6) + 1;
return zahl;
}[/code]
Die Methode gehört nicht in

Quadrat

, da sie nichts mit der Klasse zu tun hat. Außerdem holt man sich einen zufälligen int über [JAPI]java.util.Random#nextInt(int)[/JAPI].

 

[ehlr]

So, ich habe jetzt Verbesserungen angewendet.
Was haltet ihr davon ?

Starter.java

public class Starter {
	
	public static void main(String[] args) {
		new Wuerfel();
	}
}

Wuerfel.java

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class Wuerfel extends JFrame {

	/*
	 * Instanzvariablen
	 */
	
	private JButton button;
	private JPanel panel;
	private JLabel label;
	private Quadrat quadrat = new Quadrat();
	private Rechnen rechnen = new Rechnen();
	
	/*
	 * Konstruktor
	 */
	
	public Wuerfel() {
		super("Cube");
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        init();
        setResizable(false);
        setSize(100, 170);
        setLocationRelativeTo(null);
        quadrat.setPreferredSize(new Dimension(100, 100));
        setVisible(true);
	}
	
	/*
	 * Methode
	 */
	
	private void init() {
		panel = new JPanel();
		button = new JButton("Würfeln");
		label = new JLabel("0");
		label.setFont(new Font("Arial", Font.BOLD, 75));
		label.setForeground(Color.WHITE);
		quadrat.add(label);
		panel.add(quadrat);
		panel.add(button);
		getContentPane().add(panel);
		
		button.addActionListener(new ActionListener() {
			public void actionPerformed(ActionEvent event) {
				panel.repaint();
	            int zahl = rechnen.zufall();
	            label.setText(zahl +"");
	        }
	    });
	}   
}

Quadrat.java

import java.awt.Color;
import java.awt.Graphics;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class Quadrat extends JPanel{
 
	/*
	 * Instanzvariablen
	 */
	
	private Rechnen rechnen = new Rechnen();
	
	/*
	 * Methode
	 */
	
      @Override
  public void paintComponent( Graphics g )
  {      
    super.paintComponent(g);	  
    g.setColor(new Color(rechnen.zufallsFarbe(),rechnen.zufallsFarbe(),rechnen.zufallsFarbe()));
    g.drawRect(0,0,100,100);
    g.fillRect(0,0,100,100);
  }
}

Rechnen.java

public class Rechnen {

	public int zufall() {
		  int zahl;
		  zahl = (int) (Math.random() * 6) + 1;
		  return zahl;
	  }
	
	public int zufallsFarbe() {
		int random;
		random = (int) (Math.random() * 255) +1;
	    return random;
	}
}

 

[Jango]

Hi,

Umlaute in Klassen und Variablen sollte man auch eher vermeiden.

Nö - 'ue' ist auch ein Umlaut. Also präziser ausdrücken.

 

[ehlr]

Das ist doch egal?!

 

[Jango]

Nein!
Ä,ö,ü wird nicht immer richtig dargestellt - das meinte 'gman' damit.
Ae, oe, ue bedeutet das Gleiche, wird aber immer so dargestellt - also nicht Umlaute sein lassen, halt nur richtig schreiben. Auf einer englischen Tastatur gibts z.B. keine ä,ö,ü.

 

[hdi]

Wär mir neu dass man sowas wie "ue" als Umlaut bezeichnet. Ein Umlaut ist imho ein Buchstabe. Das hier sind zwei Buchstaben, u und e, weit und breit kein Umlaut darin enthalten. Ich will keine kleinkarierte Diskussion drüber starten, aber wenn du dir sicher bist würd ich mich über nen Link freuen. Man lernt nie aus

 

[Jango]

Ich will keine kleinkarierte Diskussion drüber starten,

Warum tust du es dann trotzdem?

 

[ehlr]

Gibts noch Feedbacks?

 

[hdi]

Warum tust du es dann trotzdem?

Was ich meinte ist dass ich nicht mit dir darüber streiten will wer Recht hat, weil ich auch keineswegs davon überzeugt bin dass ich Recht hab. Aber es wär mir neu, und ich hab nur darum gebeten mich aufzuklären, denn wenn ich bislang nicht die richtige Definition von Umlaut kenne - was sehr gut sein mag -, dann wär ich froh wenn sich das ändern würde.. Aber passt schon, wenn du dir auch nicht sicher bist dann musst du jetzt nicht das Internet recherchieren. Das meinte ich mit "ich will keine Diskussion starten"

 

[Onion]

Könnte dieser Thread zu den Umlauten, behilflich sein?

http://www.java-forum.org/java-basics-anfaenger-themen/129238-unicode.html

 

[xehpuk]

So, ich habe jetzt Verbesserungen angewendet.
Was haltet ihr davon ?

Du hast fast keinen Vorschlag umgesetzt.

Und ja – ae, oe und ue sind keine Umlaute.

 

[ehlr]

Ich habe eine neue Klasse für Berechnungen gemacht.
Super bei paintComponent hinzugefügt.
Umlaute durch ae, ue et cetera ersetzt.
Pack() aus dem Konstruktor entfernt.

Was soll ich noch tun?
Das war es doch alles.

Man kann doch Math.random() anstatt nextInt benutzen, wenn man es castet.
Wenn ich repaint(); weglasse, dann ändert sich die Farbe des Würfels nicht.

 

[xehpuk]

Die Methoden von

Rechnen

können statisch sein, da du auf keine Instanzvariablen zugreifen musst. Die Methodennamen würde ich mir auch noch einmal überlegen.

zufallsFarbe()

gibt keine Farbe, sondern einen int (übrigens zwischen 1 und 255) zurück.
Klar kann man auch

Math.random()

benutzen und dann zu int casten, aber das ist so, als würdest du das schreiben:

int i = (int) 0.0;

Statt:

int i = 0;

So könnte die Klasse aussehen:

import java.awt.Color;
import java.util.Random;

public class Rechnen {
	private static final Random random = new Random();
	
	public static int wuerfle() {
		return random.nextInt(6) + 1;
	}
	
	private static int gibZufaelligenRgbAnteil() {
		return random.nextInt(256);
	}
	
	public static Color gibZufaelligeFarbe() {
		final int r = gibZufaelligenRgbAnteil();
		final int g = gibZufaelligenRgbAnteil();
		final int b = gibZufaelligenRgbAnteil();
		return new Color(r, g, b);
	}
}

Die Farbe wird bei dir noch immer in

paintComponent()

erzeugt, was nicht korrekt ist. Du hast keinen direkten Einfluss darauf, wann diese Methode aufgerufen wird. So kann sich die Farbe also nicht nur nach dem Klicken des Buttons ändern, sondern auch mal so zwischendurch (minimier und maximier das Fenster mal).
Bei der Klasse

Quadrat

brauchst du nicht mal eine eigene

paintComponent()

, eigentlich die ganze Klasse nicht, da das JPanel durch

setPreferredSize()

ja schon eine Größe von 100 Pixel * 100 Pixel hat.
Besser also:
[JAVA=56]public void actionPerformed(ActionEvent event) {
quadrat.setBackground(Rechnen.gibZufaelligeFarbe());
int zahl = rechnen.wuerfle();
label.setText(String.valueOf(zahl));
}[/code]
(Außerdem siehst du dort auch, wie man einen int in einen String konvertiert.)

 

[hdi]

xehpuk,

da hat sich ein kleiner Flüchtigkeitsfehler in den Code geschlichen:

 int zahl = rechnen.wuerfle();

=> Rechnen.wuerfle();

Wollte ich nur erwähnt haben, nicht dass der TO verwirrt ist

Und als Ergänzung (auch das an den TO gerichtet):
Es gibt grundsätzlich natürlich durchaus einen Unterschied, ob man die Farbe per setBackground setzt oder selbst was in der paintComponent zeichnet. Allerdings füllst du das gesamte Panel in der Farbe, und nun rate mal was die Default-Implementierung der Methode tut: Genau das, unter Verwendung der gesetzten Background-Farbe.

D.h.: Die paintComponent musst du nur überschreiben und dort selbst mit nem Graphics-Objekt rummalen, wenn du nicht (nur) einfach das gesamte Panel in einer Farbe füllen willst, sondern zB nur ein bestimmt-dimensioniertes Quadrat reinzeichnen willst, oder noch andere Dinge.

Für deinen Anwendungsfall sollte man es genau so machen wie xehpuk es vorschlägt.

 

[ehlr]

Habe es nun so umgesetzt, danke!