Mein erstes Java Projekt - Sauberer code?

[Eisfreak7]

Hey!
Ich lerne grade Jave Programmieren mit dem Buch "Java von Kopf bis Fuß". Dabei habe ich mir zum Spaß und zur übung ein kleines Projekt vorgenommen:
Das Kartenspiel "Durak" zu schreiben. Die Regeln des Kartenspiels sind erstmal nicht so wichtig.
Ich habe jetzt grade das Kapitel über GUIs durch und habe mir auch gleich eine gebastelt, aber irgentwie habe ich das gefühl der Code ist ziemlich unsauber. Es kommt zwar in etwa das raus was ich haben wollte, aber ich bin trotzdem nicht ganz zufrieden und ich möchte mir natürlich keine unsaubere schreibweise angewöhnen.
Ich hoffe auf ein paar Verbesserungsvorschläge.

Hier die GUI wie sie bis jetzt aussieht:

Und hier der code:

import java.awt.*;
import javax.swing.*;

public class GUI {
	JComboBox handkartenCombo = new JComboBox();
	
	static{
		JFrame frame = new JFrame("Durak");
		frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
		
		String[] tischKartenNamen = {"Pik Bube", "Herz Dame"};
		JList tischKarten = new JList(tischKartenNamen);
		frame.add(tischKarten, BorderLayout.CENTER);
		JPanel spielerControlsPanel = new JPanel();
		spielerControlsPanel.setLayout(new BoxLayout(spielerControlsPanel, BoxLayout.Y_AXIS));
		frame.add(spielerControlsPanel, BorderLayout.WEST);
		JComponent[][] spielerControls = new JComponent[4][7];
		for (int i = 0; i<4; i++) {//Alle spieler durchgehen
			spielerControls[i][0] = new JPanel(new BorderLayout()); //Das Panel für den Spieler erstellen (BorderLayout)
			spielerControls[i][0].setBorder(BorderFactory.createLineBorder(Color.red)); //Den farbigen Rand zeichnen //! Farbe je nach status bestimmen
			
			JLabel tempLabel = new JLabel("Spieler " + (i+1)); //Das Label für den Spielernamen erstellen
			tempLabel.setHorizontalAlignment(SwingConstants.CENTER); //Das Label zentrieren
			spielerControls[i][0].add(tempLabel, BorderLayout.NORTH); //Das Label zum Panel hinzufügen
			
			String[] tempStringArray = {"Pik Bube", "Herz Dame"}; //! Temporär
			spielerControls[i][1] = new JList(tempStringArray); //Die Liste für die Handkarten erstellen //! Scrollbalken
			spielerControls[i][0].add(spielerControls[i][1], BorderLayout.WEST); //Die Liste zum Panel hinzufügen
			
			spielerControls[i][2] = new JPanel(new GridLayout(5, 1, 0, 1)); //Ein Panel für die Buttons erstellen
//			spielerControls[i][2].setLayout(new BoxLayout(spielerControls[i][2], BoxLayout.Y_AXIS)); //Das Layout für das Panel auf BoxLayout setzen (untereinander)
			spielerControls[i][3] = new JButton("Angreifen");
			spielerControls[i][4] = new JButton("Decken");
			spielerControls[i][4].setEnabled(false);
			spielerControls[i][5] = new JButton("Schieben");
			spielerControls[i][5].setEnabled(false);
			spielerControls[i][6] = new JButton("Schlucken");
			spielerControls[i][6].setEnabled(false);;
			spielerControls[i][2].add(spielerControls[i][3]);
			spielerControls[i][2].add(spielerControls[i][4]);
			spielerControls[i][2].add(spielerControls[i][5]);
			spielerControls[i][2].add(spielerControls[i][6]);
			spielerControls[i][0].add(spielerControls[i][2], BorderLayout.EAST);
			spielerControlsPanel.add(spielerControls[i][0]); //Das Panel auf das Spielerpanel setzen
		}
		
		
		frame.setSize(800,800);
		frame.setVisible(true);
	}
	
    private GUI() {} //Nicht instanzierbar
}

Danke für eure Hilfe

 

[JohannisderKaeufer]

der static Block und der private Constructor finde ich schon grenzwertig.

Ein paar Methoden täten dem ganzen gut.


Ansonsten würde ich mir an der Stelle auch noch das MVC-Pattern anschauen.

 

[Eisfreak7]

Wieso grenzwertig? Ich brauche doch nur eine GUI und wenn ich die statisch mache kann ich von überall drauf zugreifen

Ja das ist bis jetzt nur die gui später krigt es dann noch methoden wie addHandkarte etc. und die ganzen buttons kriegen funktionen.....
Was ist ein MVC pattern? Laut google ist mvc die trennung von programm und gui?

 

[Marc T.]

Das MVC (Model View Controller) ist ein Design-Pattern. Es sorg für eine klare Trennung
zwischen Programmlogik, Grafischer Oberfläche und dem Model (Daten).

Ein Beispiel:

1. Du drückst auf deiner grafischen Oberfläche eine Taste.
2. Die GUI teilt dem Controller mit, dass der Benutzer gerade diese Taste gedrückt hat
3. Der Controller führt eine Aktion durch
4. Der Controller teilt der GUI mit, was sie nun anzeigen soll
5. Der Controller teilt dem Model mit, das es neue Daten gibt
6. Das Model teilt der GUI mit, dass es sich geändert hat
6. Die GUI holt die neuen Daten vom Model

Edit:

Auch eine grafische Oberfläche kann und sollte man Objekt-Orientiert (das heißt nicht static)
programmieren. Aber ganz ohne static mache ich es auch nicht. So definiere ich immer meine
ganzen Buttons und Textfelder usw in einer eigenen Klasse als static, so dass ich immer
Bequem drauf zugreifen kann, ohne ständig mit Referenzen zu schupsen.

 

[Eisfreak7]

Und was sollte ich dann wegen dem MVC ändern? Ich glaub ich verstehe das nicht ganz hast du vielleicht n Beispiel?

Ok aber was nützt es mir eine Instanz von der GUI zu haben wenn ich nur eine brauche und die wichtigen sachen so wie so statisch sind?

 

[tfa]

So definiere ich immer meine
ganzen Buttons und Textfelder usw in einer eigenen Klasse als static, so dass ich immer
Bequem drauf zugreifen kann, ohne ständig mit Referenzen zu schupsen.

Das heißt, du machst gar kein MVC?

 

[Marc T.]

Bei deinem Beispiel musst du bis jetzt eigentlich nichts ändern, da du
bisher nur View-Code hast. Logik und Model hast du zumindest noch nicht
gepostet.

Das mit dem static ist eine Art Glaubenskrieg. Eine Objekt-Orientierten
Sprache ist genau das, wegen ihrer Möglichkeit Objekte zu erzeugen,
warum also das ganze System aushebeln?

Bei statischen Feldern sehe ich da eigentlich kein Problem aber gleich die
ganze Klasse static zu machen... naja ^^

Es gibt wunderschöne Patterns für Objektorientierten Programmierstil
die einen das Leben bei größeren Projekten um vieles einfacher machen.

Und zu tfa:
Entschuldige für die Ungenauigkeit, ich greife natürlich nur mit meiner GUI
auf meine GUI-Elemente zu. Da diese oft in vielen verschiedenen Klassen
der GUI benötigt werden finde ich das sehr praktisch.

 

[Fab1]

Das ist dein "erstes" eigenes Projekt, ich würde mir da nicht so viele Gedanken über einen perfekten Code machen. Wenn ich jetzt meinen Code von vor einem Jahr anschaue, dann wundere ich mich auch. Aber das macht nichts.

Des Weiteren finde ich es auch nicht hilfreich, wenn man gleich mit MVC anfängt. Vor allem nicht, wenn man noch am Anfang ist, aber das ist meine Meinung.

Natürlich sollte irgendwann der Punkt kommen, an dem man sich Sorgen über die Übersichtlichkeit seines Codes macht, aber das kannst du auch noch, wenn du mit Java von Kopf bis Fuß fertig bist.

 

[Marc T.]

Aber wenn er doch konkret nach einer Beurteilung Tipps fragt

 

[Eisfreak7]

Ja das ist meine erste objektorientierte Sprache und vorher habe ich fast nur mit AutoIT gearbeitet. Das ist sowiso eine ziemlich unsaubere sprache die viel mehr Fehler verzeiht als Java.

Wie ist es denn mit dem 2-Dimensionalen JComponent[] array das die Komponenten für die einzelnen Spieler enthält? Kann man das so machen? Oder sollte ich besser für jedes einzelnes Control ein eigenes Array erstellen? Wie Button[] karteSpielenButtons o.ä?
So wie es jetzt ist müssten die controls ja wenn man auf sie zugreifen will immer erst in einer temporärern variable gespeichert und auf den richtigen typ gecastet werden und man muss sich genau merken hinter welchem index welches control steckt. Mir ist aber keine bessere Methode eingefallen?

Edit: Mir fällt grad noch ne möglichkeit ein: Könnte ich nicht ein Objekt spielerComponentsCollection machen die Components als instanzvariablen hat und sie auf ein JPanel setzte das man mit einer methode getPanel() abrufen kann?
Werde das morgen ausprobieren.

 

[Helgon]

Vllt hilft dir das

Java SE Application Design With MVC

 

[Marco13]

@Helgon: Findest du den Artikel hilfreich? Ich find' den stilistisch irgendwie ... seltsam... vielleicht wegen des EE-Bezugs...?! :bahnhof:

@Eisfreak7: Ja, daran ist IMHO nicht sooo viel wirklich ... "gut". Das mit dem static Initializer ist Unfug, von Details wie dass das GUI auf dem EDT erstellt werden sollte mal ganz abgesehen. Aber auch innen macht vieles nicht so viel Sinn. Undn nur weil das auf dem Screenshot hübsch aussieht, heißt das nicht, dass das der richtige Weg ist, um weiterzumachen. Das mit dem Components-Array ist :autsch: schräg. Die JList-Inhalte kann man ja nicht wirklich verwenden. Die Buttons haben auch noch keine Funktion. Es fehlt zu viel, um "fundierter" Antworten zu können, aber so wirst du nicht weit kommen. Aber das ist natürlich alles erstmal subjektives Gelaber, und hilft nicht wirklich weiter.

Ein (auch nur schnell hingeschriebener) Alternativvorschlag:

package javaforum;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.*;
 
class Durak
{
    void attack(int player)
    {
        System.out.println("Player "+player+" attacks");
    }
    int getNumPlayers()
    {
        return 4;
    }
}

class DurakPanel extends JPanel
{
    private final Durak durak;
    
    public DurakPanel(Durak durak)
    {
        super(new BorderLayout());
        this.durak = durak;

        String[] tischKartenNamen = {"Pik Bube", "Herz Dame"};
        JList tischKarten = new JList(tischKartenNamen);
        add(tischKarten, BorderLayout.CENTER);
        
        JPanel spielerControlsPanels = new JPanel();
        spielerControlsPanels.setLayout(new BoxLayout(spielerControlsPanels, BoxLayout.Y_AXIS));
        add(spielerControlsPanels, BorderLayout.WEST);
        
        for (int i=0; i<durak.getNumPlayers(); i++)
        {
            JComponent spielerControlPanel = createSpielerControlPanel(i);
            spielerControlsPanels.add(spielerControlPanel);
        }
    }

    private JComponent createSpielerControlPanel(final int player)
    {
        JPanel spielerControls = new JPanel(new BorderLayout());
        spielerControls.setBorder(BorderFactory.createLineBorder(Color.red));
        spielerControls.add(new JLabel("Spieler " + (player+1), JLabel.CENTER), BorderLayout.NORTH);
            
        String[] tempStringArray = {"Pik Bube", "Herz Dame"};
        JList cardsList = new JList(tempStringArray);
        spielerControls.add(cardsList, BorderLayout.WEST);
            
        JPanel buttonPanel = new JPanel(new GridLayout(5, 1, 0, 1));
        JButton attackButton = new JButton("Angreifen");
        buttonPanel.add(attackButton);
        attackButton.addActionListener(new ActionListener()
        {
            @Override
            public void actionPerformed(ActionEvent e)
            {
                durak.attack(player);
            }
        });
        
        JButton deckenButton = new JButton("Decken");
        deckenButton.setEnabled(false);
        buttonPanel.add(deckenButton);

        JButton schiebenButton = new JButton("Schieben");
        schiebenButton.setEnabled(false);
        buttonPanel.add(schiebenButton);

        JButton schluckenButton = new JButton("Schlucken");
        schluckenButton.setEnabled(false);;
        buttonPanel.add(schluckenButton);

        spielerControls.add(buttonPanel, BorderLayout.CENTER);
        return spielerControls;
    }
}

public class GUI 
{
    public static void main(String[] args)
    {
        SwingUtilities.invokeLater(new Runnable()
        {
            @Override
            public void run()
            {
                createAndShowGUI();
            }
        });
    }
    
    private static void createAndShowGUI()
    {
        JFrame frame = new JFrame("Durak");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        
        Durak durak = new Durak();
        DurakPanel durakPanel = new DurakPanel(durak);
        
        frame.getContentPane().add(durakPanel);
        frame.setSize(800,800);
        frame.setVisible(true);
    }
}

Sowas wie MVC solltest du dir auf jeden Fall aber mal ansehen, entweder direkt bei http://www.java-forum.org/allgemeines/91829-mvc.html oder (was ich trotz einiger kleiner glitches recht hilfreich fand) sowas wie MVC Pattern

 

[Eisfreak7]

Ok. Das das mit dem Array nicht die sauberste methode ist denke ich ja auch.
Danke für das Beispiel aber was ich daran nicht verstehe ist:
1. deklarierst du die 3 klassen absichtlich alle in der selben klasse oder sollten die in eigene?
2. [c] int getNumPlayers()
{
return 4;
}[/code]
wofür ist das gut? Wieso nicht direkt 4 benutzen?
3. Wieso ist der Rückgabetyp von createSpielerControlPanel "JComponent" und nicht "JPanel"?
4. Du überschreibst die Variable "spielerControlPanel" immer wieder. Wie soll man am ende auf die 4 JLists zugreifen?
5.

            @Override
            public void actionPerformed(ActionEvent e)
            {
                durak.attack(player);
            }

Die Syntax ist mir nicht bekannt. Ist das eine Art unterklasse? Ohne namen?

 

[Marcinek]

1) Das kommt sicherlich von dem "schnell dahingeschrieben"

2) Und wenn die Anzahl der Player nun von der Console eingelesen werden soll?

3) Man sollte immer so weit wie möglich oben ansetzen: JComponent < JPanel. Damit kannst du später JPanel durch andere Klassen ändern.

Und selbst wenn sowas manchmal oversized oder unnötig erscheint. Wenn man Produkte entwickelt, dann wird sich sowas in der weiteren Entwicklung bezahlt machen.

4) -

5) Anonyme Klasse.

Zusammenfassend: Du benötigst noch sehr viel mehr Erfahrung. :toll:

 

[Marco13]

1. Es sind 3 Klassen in einer Datei. "In echt" würde man dafür 3 Dateien anlegen - so kann man es aber schneller Copy&Pasten und ausprobieren.

2. Ja, irgendwann gibt's vielleicht mal einen Startdialog: "Wie viele Spieler?" und dort kann man dann 1-10 auswählen...

3. Weil es nicht mehr als JComponent sein muss (es könnte sogar nur Component sein, aber ... das hat eher historische Gründe )

4. Das ist dein Problem ueh: Mal im Ernst: Ich hatte ja schon erwähnt, dass nicht klar ist, WO da welche Funktionalität untergebracht werden soll. Man könnte (wie bei 5.) an jede List einen (anonymen) ListSelectionListener hängen oder so ... Es wäre aber sicher auch nicht verkehrt, in betracht zu ziehen, eine eigene Klasse "PlayerPanel" zu machen, die die List als Instanzvariable speichert und ggf. noch ein paar weitere Funktionen anbietet (hint: Was ich gepostet hatte, war kein fertiges Programm )

5. Anonyme innere Klasse, ist für solche Sachen IMHO oft ganz praktisch.

 

[hüteüberhüte]

Du hast einen schwarzen Hintergrund. Das solltest du ändern. Das sieht sonst so nach Nerd aus (Leute, die lebendigen Tieren den Kopf abbeißen .)

 

[Eisfreak7]

Danke für eure tipps (vor allem Marco)
Ich hatte jetzt mal Zeit und hab die GUI (hoffentlich besser) neu geschrieben:

[Java]import javax.swing.event.*;
import java.awt.*;
import javax.swing.*;
import java.awt.event.*;

public class GUI {
// JList handkartenLists[];
Spieler[] spielerArray;


public void createGUI(){
//Die GUI erstellen
JFrame frame = new JFrame("Durak");
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
JList tischKarten = new JList();
tischKarten.setModel(new Angriff(new Spieler(1), new Spieler(2))); //!
//Die Tischkarten anzeigen
frame.add(tischKarten, BorderLayout.CENTER);
//! Direkt füllen

//Die Spieler
JPanel spielerPanels = new JPanel();
spielerPanels.setLayout(new BoxLayout(spielerPanels, BoxLayout.Y_AXIS));

// handkartenLists = new JList[spielerArray.length];

for(int i = 0; i < spielerArray.length; i++){ //Ein Schleifendurchlauf pro spieler
spielerPanels.add(createSpielerPanel(i));
}
frame.add(spielerPanels, BorderLayout.WEST);
frame.setSize(400,spielerArray.length*150);
frame.setVisible(true);
}

private JPanel createSpielerPanel(final int spielerPanelID){
JPanel spielerPanel = new JPanel(new BorderLayout());
spielerPanel.setBorder(BorderFactory.createLineBorder(Color.red)); //! Die Farbe vom aktuellen Typ des Spielers (Angreifer/Verteidiger/Unbeteiligter) abhängig machen
spielerPanel.add(new JLabel("Spieler " + (spielerPanelID+1), JLabel.CENTER), BorderLayout.NORTH);
JList handkartenList = new JList(); //Die Liste für die Handkarten zum Array hinzufügen //!handkartenLists[spielerPanelID]
handkartenList.setModel(spielerArray[spielerPanelID]);
handkartenList.addListSelectionListener(new ListSelectionListener(){
int spielerID = spielerPanelID;
@Override
public void valueChanged(ListSelectionEvent e)
{
if(e.getValueIsAdjusting()){ //Damit der code nicht 2 mal ausgeführt wird
JList source = (JList) e.getSource();
System.out.println(source.getSelectedValue() + " wurde von Spieler " + spielerID + " ausgewählt");
}
}
});
spielerPanel.add(handkartenList, BorderLayout.WEST);
//! Die stati der Buttons updaten
spielerPanel.add(createActionButtonsPanel(spielerPanelID), BorderLayout.EAST);
return spielerPanel;
}

private JPanel createActionButtonsPanel(final int buttonPanelID){
JPanel actionButtonsPanel = new JPanel(new GridLayout(4, 1));
JButton attackButton = new JButton("Angreifen");
attackButton.addActionListener(new ActionListener(){
int spielerID = buttonPanelID;
public void actionPerformed(ActionEvent e){
System.out.println("Spieler " + spielerID + " greift an!");
}
});
actionButtonsPanel.add(attackButton);
JButton defenseButton = new JButton("Decken");
defenseButton.addActionListener(new ActionListener(){
int spielerID = buttonPanelID;
public void actionPerformed(ActionEvent e){
System.out.println("Spieler " + spielerID + " deckt!");
}
});
actionButtonsPanel.add(defenseButton);
JButton schiebenButton = new JButton("Schieben");
schiebenButton.addActionListener(new ActionListener(){
int spielerID = buttonPanelID;
public void actionPerformed(ActionEvent e){
System.out.println("Spieler " + spielerID + " schiebt!");
}
});
actionButtonsPanel.add(schiebenButton);
JButton surrenderButton = new JButton("Schlucken");
surrenderButton.addActionListener(new ActionListener(){
int spielerID = buttonPanelID;
public void actionPerformed(ActionEvent e){
System.out.println("Spieler " + spielerID + " schluckt!");
}
});
actionButtonsPanel.add(surrenderButton);
return actionButtonsPanel;
}

public void setBorderColor(int Player, Color color){

}

public GUI(Spieler[] newSpielerArray) {
spielerArray = newSpielerArray;
createGUI();
}
}[/code]

besser so? Das mit den Anonymen klassen hilft enorm Das MVC hab ich glaub ich immer noch nicht vollkommen verstanden aber ich hab versucht das zu berücksichtigen.

Edit: Was für n schwarzer Hintergrund?
Und ich hab noch eine statische Klasse "Kartendeck" die im Prinziep nichts macht als ein Array mit Karten zu erstellen und dieses zu bearbeiten. Da es im ganzen spiel nur ein Kartendeck gibt hab ich das auch statisch gemacht. Ist das auch schlecht? Falls ja, wofür sind statische Klassen/Methoden/Variablen dann überhaupt gut?

 

[Marco13]

Static fields (außer public static final KONSTANTEN) sind immer schlecht.

Zum Code... der sieht schon aufgeräumter aus, aber einige Fragen bleiben noch

 JPanel spielerPanel = new JPanel(new BorderLayout());
            spielerPanel.setBorder(BorderFactory.createLineBorder(Color.red)); //! Die Farbe vom aktuellen Typ des Spielers (Angreifer/Verteidiger/Unbeteiligter) abhängig machen
...
handkartenList.setModel(spielerArray[spielerPanelID]);

Nur EIN Beispiel: Um die Randfarbe nachträglich zu ändern, muss 'spielerPanel' als Instanzvariable gespeichert werden.

Und ... 'Spieler' IST EIN ListModel? Das sollte wohl auch nicht so sein...

 

[Eisfreak7]

Aber static methoden werden doch auch von der Java API benutzt :O

Ja das mit dem Panel rand ist noch auf meiner Todo liste (Wie alles das mit //! gekennzeichnet ist).

Doch Spieler ist (unter anderem) ein ListModel.
Spieler hat die Informationen über die Handkarten und gibt diese an die List weiter. Oder sollte ich lieber eine Eigene Klasse für die Handkarten machen?
Hier der source von Spieler:

[Java]
import java.util.ArrayList;
import java.io.*;
import javax.swing.DefaultListModel;

public class Spieler extends DefaultListModel {
private ArrayList<Karte> Handkarten = new ArrayList<Karte>();
// private String Name;
private int ID;

@Override
public Object getElementAt(int index){
return Handkarten.get(index);
}

@Override
public int getSize(){
return Handkarten.size();
}

public Spieler(int ID) {
// setName("Name"); //! Namen abfragen
setID(ID);
nachZiehen();
}

public void setID(int setID){
ID = setID;
}

public int getID(){
return ID;
}

// public void setName(String setName){
// Name = setName;
// }
// public String getName(){
// return Name;
// }

public void nachZiehen() {
for(int i = Handkarten.size(); i<6; i++) {
if (Kartendeck.getDeckSize() > 0)
Handkarten.add(Kartendeck.KarteZiehen(Kartendeck.getDeckSize()-1));
else
break;
}
}

public void angriff(Spieler naechsterSpieler) {
Spieler uebernaechsterSpieler = this; //!
//Nach Aktion fragen
Angriff Angriff = new Angriff(naechsterSpieler, uebernaechsterSpieler);
String input = "";
do {
//Handkarten ausgeben
System.out.println("Ihre Handkarten:");
for(int i = 0; i<Handkarten.size(); i++)
System.out.println(i + ": " + Handkarten.get(i).getType() + " " + Handkarten.get(i).getValue());
System.out.println("Welche Karte möchten sie spielen?");
input = getInput();
int zuSpielendeKarte = Integer.parseInt(input);
Angriff.addAttackCard(Handkarten.remove(zuSpielendeKarte));
} while(Angriff.getState() == false);

}

public String getInput(){
String eingabeZeile = "";
try {
BufferedReader is = new BufferedReader(new InputStreamReader(System.in));
eingabeZeile = is.readLine();
if (eingabeZeile.length() == 0)
eingabeZeile = "";
} catch(IOException e) {
System.out.println("IOException: " + e);
}
return eingabeZeile;
}

public int getAction(Angriff Angriff){
int Action = 0;
return Action;
}

public int getReaction(Angriff Angriff){
System.out.println("Spieler " + this.getID() + " wird angegriffen. Folgende Karten liegen auf dem Tisch:");
ArrayList<Karte[]> AttackCards = Angriff.getAllCards();
String AttackCard;
String DefCard;
for (int i = 0; i < AttackCards.size(); i++){
AttackCard = AttackCards.get(i)[0].getType() + " " + AttackCards.get(i)[0].getValue();
if (AttackCards.get(i)[1] == null)
DefCard = "Nicht gedeckt";
else
DefCard = AttackCards.get(i)[1].getType() + " " + AttackCards.get(i)[1].getValue();
System.out.println(i + ": " + AttackCard + " - " + DefCard);
}
System.out.println("Wählen sie eine Option:");
System.out.println("0: Karte decken");
System.out.println("1: Karten schieben");
System.out.println("2: Karten schlucken");
String input = getInput();
int Aktion = 0;
if (Aktion == 0){ //!Karte decken
System.out.println("Welche karte möchten sie decken?");
int deckKarte = Integer.parseInt(getInput());
System.out.println("Mit welcher Karte möchten sie das tun?");
int handKarte = Integer.parseInt(getInput());
Angriff.karteDecken(deckKarte, Handkarten.remove(handKarte));
}
else if (Aktion == 1){ //!Karten schieben
}
else if (Aktion == 2) { //Karten schlucken
Angriff.setFinished();
for(Karte[] Kartenpaar:Angriff.getAllCards())
for(Karte eineKarte:Kartenpaar)
if (eineKarte != null)
Handkarten.add(eineKarte);
}
return Aktion;
}

public String toString(){
return "Spieler " + getID();
}

}
[/Java]
Der code ist natürlich noch konsolen-basiert und muss noch abgeändert werden. Aber er erweitert ListModel.

 

[Marco13]

Static Methoden sind gut und praktisch und nützlich (verwende ich selbst so gerne, dass andere es schon "zu viel" finden). Nur static fields (Variablen) sind schlecht, bzw. man sollte sie höchst-kritisch hinterfragen.

Ein Spieler IST kein ListModel. (DU bist vielleicht manchmal ein Spieler. Und, bist du dann ein ListModel? ). Ich kenne die Regeln und Abläufe in diesem Spiel nicht. Und ob es angebracht wäre, die Karten statt mit einem String lieber mit einer eigenen "Card"-Klasse zu modellieren, weiß ich nicht, aber das könnte gut sein (ein String ist zu "schwammig", der kann ja alles mögliche sein). Auf jeden Fall sollte der Spieler aber KEIN ListModel sein, sondern (quasi unabhängig von GUI sein und) z.B. eine Methode haben wie

// Returns an unmodifiable list containing the cards currently held by this player...
List<String> getCurrentCards()
{
...
}

Auch solltest du überlegen, ob wirklich so viel Spiellogik direkt in der Spieler-Klasse liegen sollte. Eine Herausforderung (bzw. schon fast ein Tipp im hinblick auf MVC) : Versuch' mal, deine Klassen so zu Modellieren, dass man sie wahlweise über das GUI oder über die Console steuern kann, ohne dass die Klassen selbst dafür geändert werden müssen. (Das ist keine Notwendigkeit, aber eine Möglichkeit, sich zu einer bestimmten Form von Disziplin zu zwingen )

 

[Eisfreak7]

Gut dann werde ich für die TischKarten und die HandKarten jeweils eine eigene Klasse machen und diese sind dann ListModels, richtig? Die werden dann von der GUI und von der Klasse spieler gesteuert.
Die Klasse spieler kann doch eigentlich nur was sie können muss? Sie hat Methoden um ihre Handkarten auszuspielen, sie also "weiterzugeben".
Ich überarbeite aber die Spieler Klasse gerade allgemein ein bisschen.
Es existiert eine Klasse Karte. Nur zur Anzeige verwende ich einen String, da der user ja wenig mit der genauen Heap Position des Karten-Objekts anfangen kann
Bei der Steuerung der Konsole bin ich an meine grenzen geraten da Teilweise mehrere Spieler gleichzeitig zum Handeln aufgefordert werden müssten.

[Java]
public class Karte {
private int ValueID;
private String Value;
private int TypeID;
private String Type;

public Karte(int CardValue, int CardType){
this.setValue(CardValue);
this.setType(CardType);
}

public int getValueID() {
return(ValueID);
}
public String getValue() {
return(Value);
}
public void setValue(int SetValue) {
ValueID = SetValue;
if (ValueID <= 10)
Value = Integer.toString(ValueID);
else if (ValueID == 11)
Value = "Bube";
else if (ValueID == 12)
Value = "Dame";
else if (ValueID == 13)
Value = "König";
else if (ValueID == 14)
Value = "Ass";
}

public int getTypeID() {
return(TypeID);
}
public String getType() {
return(Type);
}
public void setType(int setType) {
TypeID = setType;
if (TypeID == 1)
Type = "Kreuz";
else if (TypeID == 2)
Type = "Pik";
else if (TypeID == 3)
Type = "Herz";
else if (TypeID == 4)
Type = "Karo";
}

public String toString(){ //Den Kartennamen ausgeben, z.B. Karo Bube
return getType() + " " + getValue();
}
}
[/code]

 

[Marco13]

Für die Tisch- und Handkarten sollte es vermutlich (!) nur eine Klasse "Karte" geben. Sind ja beides Karten, und können vermutlich auch von der Hand auf den Tisch wandern (oder umgekehrt) - dadurch ändert sich ja der Typ nicht wirklich.

Statt der ganzen Strings dort könntest du auch Enums verwenden. Zufälligerweise war genau das (Suit und Rank) das Beispiel von Enums (Ob das wirklich passt, musst du beurteilen!)

 

[Eisfreak7]

Die enums klingen nützlich sehe ich das richtig das man damit eigentlich einfach nur Zahlen Strings zuordnet? Also ich sage einfach nur
2=Wert
Und kann dann auswählen ob ich 2 oder Wert haben will.

Ja die Karten können wandern aber Handkarten ist doch keine Karte und Karte hat auch keine Handkarten genau so ist es mit Tischkarten oder nicht?

Die Klasse HandKarten sieht bei mir jetzt so aus:


[Java]
import java.util.ArrayList;
import javax.swing.*;

public class HandKarten extends DefaultListModel{
private ArrayList<Karte> handkarten = new ArrayList<Karte>();

@Override
public Object getElementAt(int index){
return handkarten.get(index);
}

@Override
public int getSize(){
return handkarten.size();
}

public Object getCard(int index){ //Übergibt die karte
return handkarten.remove(index);
}

public void addCard(Karte karte){
handkarten.add(karte);
}
}
[/code]

 

[Marco13]

Ah, ich dachte du meinst sowas wie

class Handkarte { ... }
class Tischkarte { ... }

aber so wie du es jetzt beschrieben hast ist das was anderes. Ich denke, dass auch diese Mengen (Hand- bzw. Tischkarten) keine ListModel sein sollten oder müßten. Hast du dir schon irgendein Modell für das Spiel selbst überlegt? (Ich hatte da ja so eine "class Durak" angedeutet...)

 

[Eisfreak7]

Wieso sollten das denn keine ListModels sein? Sie enthalten doch nur informationen und methoden um diese zu verwalten. Was sollte ich denn stattdessen als ListModel nehmen?
Ich weiß nicht was du meinst. Was für eine class Durak? Wofür soll die da sein?

 

[Marco13]

Es ist (bei so einem kleinen Spiel vielleicht nicht so wichtig, aber im allgemeinen) nicht verkehrt, sich erstmal darüber im Klaren zu sein, was man machen will. Zumindest würde ich niemandem, der ein Programm schreiben will, vorschlagen: "Erstell' erstmal ein Fenster mit Buttons drin, und ändere dann so lange am Code rum, bis er macht, was er soll" Vielleicht wäre ein Blick auf http://www.java-forum.org/allgemeines/91829-mvc.html (bzw. den ersten Post dort) nicht so verkehrt. Jedenfalls hatte ich so eine "class Durak" in http://www.java-forum.org/java-basi...es-java-projekt-sauberer-code.html#post954215 angedeutet (!). Es geht darum, dass man sich ja überlegen kann: "Es gibt das Spiel Durak, mit Player, die Player haben Karten, jeder Player kann einen Agriff machen und dabei Karten auf den Tisch legen....

class Durak {
    List<Player> getPlayers() { ... }
}
class Player {
    List<Card> getCards() { ... }
    List<Card> getCardsForAttack() { ... }
}
...

In dieser Phase überlegt man sich nicht unbedingt zuerst, ob ein JPanel nun einen roten oder grünen Rand hat. Allerdings ist der Wunsch, schnell irgendwas buntes, anklickbares auf dem Bildschirm zu haben, nachvollziehbar, deswegen muss man die Planung vielleicht gar nicht so nazihaft-abstrakt machen - das wird ja kein Framework, auf das sich 10 Jahre lang weitere Entwicklungen stützen werden, und auch bei "pragmatischerer" Vorgehensweise lernt man automatisch was (und sei es nur, was man hätte besser machen können )

Zu den ListModels: Ich bin mir nicht sicher, wo und wie du genau diese Klassen verwenden wolltest. Es gibt aber mehrere Dinge, die daran "nicht so ganz passen":

- Eine eigene Klasse für eine Menge von Karten zu definieren ist hinterfragenswert, solange diese Klasse keine weitere Funktion hat. Oder ganz direkt: Statt

class Handkarten extends DefaultListModel { ...} 
class Tischkarten extends DefaultListModel { ...}

könntest du so gesehen auch einfach

class KartenMenge extends DefaultListModel { ...} 
...
KartenMenge handkarten = new KartenMenge();
KartenMenge tischkarten = new KartenMenge();

machen. Aaaber...:

- Es gibt eigentlich keinen Grund, DefaultListModel zu erweitern. Ein DefaultListModel IST so gesehen schon eine "KartenMenge" (bzw. eine allgemeine Menge von Objekten). Man braucht also keine eigene ArrayList mehr für die Karten. Das DefaultListModel HAT selbst schon so eine Liste. Wenn du zusätzlich eine eigene Liste erstellst, und einige Methoden überschreibst, wird das Model nicht mehr richtig funktionieren. Das DefaultListModel ist so gebaut, dass man es einer JList übergibt, und die JList dann dieses Modell anzeigt. Das heißt: Wenn man dem Model ein Element hinzufügt (mit 'addElement' - nicht mit einer eigenen Methode!) dann wird automatisch die JList aktualisiert, so dass die das neue Element auch anzeigt. Bei deinen Klassen würde das nicht funktionieren.

- Die Mengen der Tisch/Handkarten "nur" in einem DefaultListModel zu speichern könnte man als unschön betrachten. Das ist jetzt vielleicht im ersten Moment schwer nachzuvollziehen, aber ich will es erwähnen: Damit legt man sich auf eine JList in Swing fest. Man legt fest, dass das (eigentlich abstrakte und allgemeine) Spiel, das man da schreibt, IMMER mit Swing in Verbindung stehen wird, und die Kartenmengen IMMER in einer JList angezeigt werden. Was passiert, wenn du die Karten in Zukunft in einer JTable anzeigen willst? Ja, die Klassen, die eigentlich ganz allgemein "das Spiel" beschreiben sollten, müssen (u.U. grundlegend) geändert werden.

Der letzte Punkt erscheint dir im Moment vielleicht nicht so kritisch - und eigentlich ist er das auch nicht: Es ist klar, dass das nur ein kleines Spiel wird, dass du "zum Üben" schreibst, und dass es immer Swing sein wird, und dass die Karten eben in einer JList stehen werden. Punkt.
Natürlich ist es für so etwas sehr einfach, zu schreiben

class Durak
{
    public DefaultListModel getHandkarten() {...}
    public DefaultListModel getTischkarten() {...}
}

weil man diese Models dann einfach in JLists anzeigen kann, und bei einem

durak.getHandkarten().addElement(neueHandkarte);

wird automatisch die JList aktualisiert - egal, von wo und wann diese Methode aufgerufen wird (solange es aus dem richtigen Thread heraus passiert ).

Wenn man diese "enge Anbindung an Swing/JList" vermeiden wollte, müßte man sich über eigene Listener und Events Gedanken machen, was sehr aufwändig sein kann. Aber ... wenn man (später mal) komplexere Programme schreibt, muss man das - man könnte es also etwas polemisch auf die Frage runterbrechen, was du üben willst: Swing oder Programmieren? (Nicht so ernst nehmen )

 

[Eisfreak7]

Ja das Problem bei diesem Projekt ist das ich das halt nebenbei geschrieben habe (Ich habe irgentwann einfach mal angefangen, eine Karten klasse zu machen. Dann hab ich mir nen Spieler gemacht. Etc. Und immer wenn mir neue Möglichkeiten klar waren habe ich die Hinzugefügt. Als ich mit dem ganzen angefangen habe war mir noch nicht klar das es sowas wie Intefaces etc. gibt
Ich habe mir den ersten Post mal durchgelesen und bin an deinem grad dran, hab aber momentan nicht genug Zeit den zuende zu lesen (echt ausführliches tut (thumbsup)).
Ich habe bis jetzt eine class spiel die war aber mehr dazu da den ablauf des spiels konsolenbasierend zu steuern.
Ist es nicht manchmal auch ganz sinnvoll mit dem View anzufangen? Dabei kann man sich nochmal genau darüber klar werden was model & controller überhaupt können müssen und man hat direkt was zum testen.
Ich verwende zum Beispiel die Klasse Handkarten momentan als Instanzvariable von Spieler und als ListModel der HandkartenList.
Ich habe schon drüber nachgedacht eine "KartenSammlung" klasse als Oberklasse zu machen von der die Handkarten etc. erben, aber nur so eine Klasse würde nicht reichen da z.B. die Tischkarten 2-Dimensional sind und andere Methoden brauchen als die Handkarten.

Jetzt habe ich das Prinzip von MVC denke ich auch mehr oder weniger verstanden. Das heißt also ich soll Pro Objekt (Wie z.B. einem SpielerPanel) 5 Klassen erstellen? View, Controller, Modell, Listener, Event? Wird das nicht n ziemliche Haufen Klassen? Ist es nicht unschön so viele Klassen für ein Ding zu haben?
Und nebenbei: Wofür ist swingUtilitys.invokelater gut? Google hat mir nicht wirklich geholfen
Ich denke ich muss mein Programm nochmal von Grund auf überdenken :autsch:

 

[Marco13]

Neee, lass dich mal nicht zuuu sehr beirren. Wie gesagt, das wird ja nichts, was über Jahre weitergepflegt wird oder so - da kann man wohl an einigen Stellen pragmatischer sein.

Man würde, wenn überhaupt, diese Klassen ausgehend vom Modell erstellen. Und im Zweifelsfall könnte man das auch sehr "grobgranular" machen, dass man praktisch nur einen "GameListener" hat, mit einer unspezifischen "gameStateChanged"-Methode, mit der dann die View aktualisiert wird. Allgemein ist das, worum es bei MVC geht, der Punkt dass man das Modell beschreibt, wie es ist, und dass diese Beschreibung weitgehend unabhängig von der View/GUI ist. Es gibt aber keine "strikte Vorschrift", wie genau das auszusehen hat, man hat da viele Freieheitsgrade (wie man an den immer wieder auftauchenden Diskussionen hier zu MVC auch sieht )


SwingUtilities.invokeLater bewirkt, dass das übergebene Runnable auf dem "Swing-Thread" (dem Event Dispatch Thread) ausgeführt wird, der sich um ALLES kümmert, das mit dem GUI zu tun hat.

 

[bERt0r]

Einen eigenen Listener und ein eigenes Event musst du nur selten erstellen. Z.B bei Karte. Was kann eine Karte? Ich nehm einfach mal an, sie hat einen Wert, eine Farbe und sie kann offen oder verdeckt liegen.

Da haben wir unser Model class Karte, Variablen wert, farbe und istVerdeckt. Noch Get-Funktionen und eine setVerdeckt Funktion hinzugefügt und wir haben ein Model.

Unsere View ist jetzt wohl irgendein JComponent, der ein Objekt der Klasse Karte erhält, Wert und Farbe abfrägt und dementsprechend ein schönes Bild malt.

Wollen wir jetzt, dass die Karte auf/zugedeckt wird wenn man draufklickt, benötigen wir einen Controller. In dem Fall reicht uns ein MouseListener, den wir an die View hängen. Der ruft dann bei unserem "Karte"-Model die Funktion setVerdeckt auf.

Jetzt steht man nur vor dem Problem, wie man der View sagt, dass sie auf die Änderung im Model reagieren muss. Eine Möglichkeit ist das Observer Pattern. Du kannst das Model von Observeable erben lassen und die View Observer implementieren lassen. Du kannst dir auch einen eigenen Listener bauen und deine View über diesen am Model "lauschen" lassen. Du kannst auch deinem Controller eine Referenz auf die View geben (was ich aber nicht für sauberes MVC halte, weils dann Probleme mit mehreren Views gibt).