3. Verbesserungsvorschläge/Feedback zu meinem ersten Programm

Verbesserungsvorschläge/Feedback zu meinem ersten Programm

  • ?

    hi!

    seit 2 wochen lerne ich mit jesse liberty's buch c++ und habe gestern mein erstes eigenes programm geschrieben. es ist ein kleines altbekanntes spielchen. wer will kann das programm kompilieren und mal ausprobieren und mir dann verbesserungsvorschläge/feedback zu kommen lassen. im code könnte ich die && stellen noch mit einer schleife kompakter aufschreiben (siehe meinen anderen thread) und die ausgabe könnte ich mit leeren zeilen noch etwas auflockern.

    #include <iostream>
using namespace std;

enum STATE {empty, stateX, stateO};
enum WIN {continuegame, draw, playerX, playerO};

class board
{
 public:
  board();
  ~board(){};
  bool insert(STATE, int);
  WIN check() const;
  void drawboard() const;

 private:
  void setboardstate(int,int);
  char convtosymb(int, int) const;
  STATE boardstate[5][5];
};

board::board()
{
 for(int i=0; i<5; i++)
 {
  for(int j=0; j<5; j++)
   boardstate[i][j]=empty;
 }
}

bool board::insert(STATE coin, int column)
{
 bool wronginput;
 wronginput = (column<1) || (column>5) ||
              (boardstate[0][column-1]!=empty &&
               boardstate[1][column-1]!=empty &&
               boardstate[2][column-1]!=empty &&
               boardstate[3][column-1]!=empty &&
               boardstate[4][column-1]!=empty);
 if(wronginput)
 {
  cout << "Falsche Eingabe!" << endl;
  return false;
 }
 else
 {
  for(int i=4; i>-1; i--)
  {
   if(boardstate[i][column-1]==empty)
   { 
    boardstate[i][column-1]=coin;
    break;
   }
  }
  return true;
 }
}

char board::convtosymb(int row, int column) const
{
 if(boardstate[row][column]==empty)
  return ' ';
 if(boardstate[row][column]==stateX)
  return 'X';
 else
  return 'O';
}

void board::drawboard() const
{
 cout << " 1 2 3 4 5 " << endl;
 for(int i=0; i<5; i++)
 {
  for(int j=0; j<5; j++)
  { 
   cout << "|" << convtosymb(i,j);
  }
  cout << "|" << endl;
 }
}

WIN board::check() const
{
 //horizontal
 for(int i=0; i<5; i++)
 {
  for(int j=0; j<3; j++)
  {
   if(boardstate[i][j]==stateX &&
      boardstate[i][j+1]==stateX &&
      boardstate[i][j+2]==stateX)
    return playerX;
   if(boardstate[i][j]==stateO &&
      boardstate[i][j+1]==stateO &&
      boardstate[i][j+2]==stateO)
    return playerO;
  }
 }

 //vertikal
 for(int i=0; i<3; i++)
 {
  for(int j=0; j<5; j++)
  {
   if(boardstate[i][j]==stateX &&
      boardstate[i+1][j]==stateX &&
      boardstate[i+2][j]==stateX)
    return playerX;
   if(boardstate[i][j]==stateO &&
      boardstate[i+1][j]==stateO &&
      boardstate[i+2][j]==stateO)
    return playerO;
  }
 }

 //diagonal nach rechts oben
 for(int i=2; i<5; i++)
 {
  for(int j=0; j<3; j++)
  {
   if(boardstate[i][j]==stateX &&
      boardstate[i-1][j+1]==stateX &&
      boardstate[i-2][j+2]==stateX)
    return playerX;
   if(boardstate[i][j]==stateO &&
      boardstate[i-1][j+1]==stateO &&
      boardstate[i-2][j+2]==stateO)
    return playerO;
  }
 }

 //diagonal nach rechts unten
 for(int i=0; i<3; i++)
 {
  for(int j=0; j<3; j++)
  {
   if(boardstate[i][j]==stateX &&
      boardstate[i+1][j+1]==stateX &&
      boardstate[i+2][j+2]==stateX)
    return playerX;
   if(boardstate[i][j]==stateO &&
      boardstate[i+1][j+1]==stateO &&
      boardstate[i+2][j+2]==stateO)
    return playerO;
  }
 }

 //unentschieden
 if(boardstate[0][0]!=empty && boardstate[0][1]!=empty &&
    boardstate[0][2]!=empty && boardstate[0][3]!=empty &&
    boardstate[0][4]!=empty && boardstate[1][0]!=empty && 
    boardstate[1][1]!=empty && boardstate[1][2]!=empty && 
    boardstate[1][3]!=empty && boardstate[1][4]!=empty && 
    boardstate[2][0]!=empty && boardstate[2][1]!=empty && 
    boardstate[2][2]!=empty && boardstate[2][3]!=empty && 
    boardstate[2][4]!=empty && boardstate[3][0]!=empty && 
    boardstate[3][1]!=empty && boardstate[3][2]!=empty && 
    boardstate[3][3]!=empty && boardstate[3][4]!=empty && 
    boardstate[4][0]!=empty && boardstate[4][1]!=empty && 
    boardstate[4][2]!=empty && boardstate[4][3]!=empty && 
    boardstate[4][4]!=empty)
  return draw;

 return continuegame;
}

int main()
{
 board game;
 int column;
 game.drawboard();

 while(true)
 {
  while(true)
  {  
   cout << "Spieler X gebe Spalte ein: ";
   cin >> column;
   if(game.insert(stateX, column))
    break;
  }
  game.drawboard();
  switch(game.check())
  {
   case draw: cout << "Unentschieden!";
              return 0;
   case playerX: cout << "Spieler X gewinnt!";
                 return 0;
   case playerO: cout << "Spieler O gewinnt!";
                 return 0;
   case continuegame: break;
   default: break;
  }

  while(true)
  {
   cout << "Spieler O gebe Spalte ein: ";
   cin >> column;
   if(game.insert(stateO, column))
    break;
  }
  game.drawboard();
  switch(game.check())
  {
   case draw: cout << "Unentschieden!";
              return 0;
   case playerX: cout << "Spieler X gewinnt!";
                 return 0;
   case playerO: cout << "Spieler O gewinnt!";
                 return 0;
   case continuegame: break;
   default: break;
  }
 }
}
    0
  • _

    Was mir direkt ins Auge springt: du rückst ja nur ein Zeichen ein! Kommst du denn damit klar? Das finde ich ein bisschen arg wenig. Imho lässt sich die Struktur viel klarer erkennen, wenn man wenigstens 2 Zeichen einrückt.

    0
  • Mod

    Sieht gut aus. 👍

    0
  • U

    auf die schnelle ist mir nur das hier aufgefallen:
    ~board(){};

    genau das wird auch vom compiler generiert, wenn du die zeile weglässt

    allerdings ist das hier alles ein wenig schwer zu erkennen(nur ein zeichen einrücken ist mir persönlich zu wenig) - dazu kommt noch, dass du die code-tags und nicht die cpp-tags genommen hast.

    bb

    0
  • V

    Anzahl gesetzter Steine zählen in einem Attribut macht unentschieden entschieden netter.

    0
  • D

    Sieht eigentlich gut aus, aber mir ist noch das hier aufgefallen:

    class board
{
 public:
  board();
  ~board(){};
  bool insert(STATE, int);
  WIN check() const;
  void drawboard() const;

 private:
  void setboardstate(int,int);
  char convtosymb(int, int) const;
  STATE boardstate[5][5];
};

    Du gibst hier deinen Parametern gar keine Namen. Versuch dich einfach mal in die Lage zu versetzen, dass du dich in eine Library einarbeiten sollst und dann haben die Parameter nicht einmal Namen. Was soll z.B das erste int sein? Pfirsichkerne? Taschenlampen oder doch orientalisch Teppiche? Du siehst, worauf ich hinaus will. 😉
    Also schön brav immer passende Namen geben und alle sind zufrieden.

    0
  • S

    unskilled schrieb:

    auf die schnelle ist mir nur das hier aufgefallen:
    ~board(){};

    genau das wird auch vom compiler generiert, wenn du die zeile weglässt

    Würde der Compiler nicht das hier erstellen?

    virtual ~board();

    Der Destructor sollte immer virtual sein. Ausnahmen bestätigen die Regel. Grund hierfür ist die Vererbung. Beim zerstören eines Objektes werden nur dann die Destructors des Basisobjektes aufgerufen, wenn dieser virtual ist. Lediglich Klassen, von denen keine Ableitungen erstellt werden dürfen, ist es Ratsam den Destructor nicht virtual zu setzen.

    Gruß,
    Thomas

    0
  • D

    Nein!

    virtual wird nirgends implizit gesetzt. Das muss man von Hand tun. Der Compilier generiert einen leeren Destruktor, wie unskilled gesagt hat.

    0
  • O

    Siassei schrieb:

    Der Destructor sollte immer virtual sein.

    Nein.

    Ausnahmen bestätigen die Regel.

    Vererbung ist die Ausnahme, nicht die Regel. Es gibt ein Vielfaches mehr an Klassen die niemals vererbt werden. Und selbst dann ist der virtuelle Destruktor nur relevant, wenn dann noch zusätzlich delete auf einen Basisklassenzeiger aufgerufen werden soll.

    Die Faustregel ist: hat eine Klasse eine Methode, die virtual deklariert wurde, dann sollte auch der Destructor virtual sein (Ausnahmen bestätigen die Regel). Sonst nicht.

    0
  • G

    drakon schrieb:

    Der Compilier generiert einen leeren Destruktor, wie unskilled gesagt hat.

    Wenn er leer ist, wozu wird er dann generiert? 😕

    0
  • D

    general bacardi schrieb:

    drakon schrieb:

    Der Compilier generiert einen leeren Destruktor, wie unskilled gesagt hat.

    Wenn er leer ist, wozu wird er dann generiert? 😕

    Das darfst du nicht so wörtlich nehmen. Das Verhalten ist einfach so, als hätte man selbst einen leeren Destruktor. Sprich es werden alle automatischen Objekte zerstört und es wird so alles aufgeräumt.

    0
  • G

    drakon schrieb:

    general bacardi schrieb:

    drakon schrieb:

    Der Compilier generiert einen leeren Destruktor, wie unskilled gesagt hat.

    Wenn er leer ist, wozu wird er dann generiert? 😕

    Das darfst du nicht so wörtlich nehmen. Das Verhalten ist einfach so, als hätte man selbst einen leeren Destruktor. Sprich es werden alle automatischen Objekte zerstört und es wird so alles aufgeräumt.

    Also quasi als Gedächtnisstütze. OK, wer es braucht... 🤡

    0
  • D

    Naja. Es ist von semantischer Bedeutung. Da ist nicht einfach kein Destruktor, sondern ein leerer. 😉
    Schliesslich muss ja jemand das ganze wieder sauber machen, nachem du etwas gebraucht hast.

    0

  • volkard schrieb:

    Anzahl gesetzter Steine zählen in einem Attribut macht unentschieden entschieden netter.

    Dafür hat man dann redundanten State, den man immer korrekt updaten muss. In Fällen wo die Geschwindigkeit keine Rolle spielt, würde ich eher in einer Schleife durchzählen, wenn ich wissen muss ob noch ein Feld frei ist.

    Natürlich sind beide Varianten in diesem einfachen Beispiel sehr einfach und übersichtlich korrekt zu implementieren.

    0
  • ?

    danke für die vorschläge, werde versuchen sie umzusetzen

    0
Log in to reply