Preview: Mein Tutorial::TicTacToe

[Shade Of Mine]

Hallo Leute.

Es wäre sehr nett von euch, wenn ihr euch mein TicTacToe-Spiel ansehen würdet -> http://tutorial.schornboeck.net/tictactoe_design.htm

Es soll ein schön designtes Spiel sein, an dem sich der Leser das eine oder andere abschauen kann. Deswegen ist es mir wichtig, dass zumindestens die Mehrheit hier das Spiel OK findet.

Der Text den ich bisher dazu geschrieben habe ist nur eine Alpha-Version - da ich erst eure Kritik abwarten will (und somit etwaige Design Änderungen) bevor ich genau das Deisgn und die Implementierung erkläre.

Weiters suche ich noch Leute die Visualisierungen dazu schreiben: Visualisierung und Programmlogik sind (so hoffe ich zumindest) schön getrennt - es wäre nett wenn jemand Visualisierungen für DirectX, OpenGL, oder bunte Konsole, oder sonstwas schreiben würde (sein Name wird natürlich erwähnt werden).

uU muss man deswegen main.cpp ebenfalls in die Visualisierung auslagern, weil unter Windows Fensterprogramme ja WinMain statt main verwenden. Ist aber nicht tragisch, da main() sowieso quasi nix macht.

[Asgard]

Hi,
ich habe gerade versucht, den Sourcecode heruntergeladen, aber leider meldet mir mein Packprogramm (iceows) immer einen CRC-Fehler.

[Shade Of Mine]

Bei mir funktionierts...

Naja, ich habe es jetzt durch eine *.zip Datei ersetzt - geht es jetzt?

[Asgard]

Jo, jetzt geht's auch bei mir.

[Shade Of Mine]

Asgard schrieb:

Jo, jetzt geht's auch bei mir.

thx, dass du es dir ansiehst!

[Asgard]

Hi,

ich habs mir mal angesehen, meiner Meinung nach sieht's ziemlich gut aus, allerdings habe ich wahrscheinlich nicht die nötige Kompetenz, um darüber wirklich urteilen zu können. Aber ich persönlich könnte mir da sicher noch ne ganze Scheibe abschneiden.

Ich hab übrigens eine kleine Visualisierung dazu geschrieben (bunte Konsole), da ich sowieso gerade dabei war, mir eine Klasse zu schreiben, die sowas wie Fenster in der Konsole simuliert. Da konnte ich das gleich mal ausprobieren. Wenn Dich die Visualisierung (nichts großes) interessiert, kann ich sie Dir gerne schicken.

[Shade Of Mine]

Jo, thx!

Ich bitte darum:

toni@schornboeck.net

Bitte schreibe deinen realname oder sonstige Sachen dazu, die ich angeben soll.

[?????]

Was hat der do_ prefix vor manchen Memberfunktionen für einen Sinn?

[Gast]

Haste schön gemacht.

Aber folgendes verstehe ich nicht:

void invalidPosition() { do_invalidPosition(); }
  void initialize() { do_initialize(); }
  void update(Position pos) { do_update(pos); }
  void winner(Type element) { do_winner(element); }
  Position getHumanAction() { return do_getHumanAction(); }
  void setField(Field const* field) { this->field=field; }

Was ist da genau der tiefere Sinn?

[Shade Of Mine]

http://tutorial.schornboeck.net/tictactoe_design.htm schrieb:

Hier wird das 'Template Method' Design Pattern verwendet - auch wenn der Nutzen in diesem speziellen Fall nicht allzugroß ist. Wir betreiben hier Overengineering - wir versuchen unsere kleine Anwendung so zu entwickeln, als wäre es eine große und komplexe Anwendung.

Es ist einfach Overengeneering Template Method hat hier wirklich keinen tieferen Sinn - allerdings versuche ich immer virtuelle Methode aus dem public Interface rauszuhalten. Es kostet ja keine Performance und erhöht die Komplexität nur minimal - dafür bin ich flexibler gegenüber Änderungen. (Auch wenn es hier keinen tieferen Sinn macht)

[ChrisM]

Hi,

OK, nicht unbedingt was Neues für Fortgeschrittene/Profis, aber ich schließe mich der Frage von Optimizer an.

ChrisM

[Gast]

Übrigens, die KI ist lausig. :p
Hast du Interresse daran, dass ich mich an einer AIPlayer-Klasse versuche?

[volkard]

ich schau mal drüber.
als der erste bildschirm voll ist schonmal kommentar...
dann kömmt:

#include "application.hpp"

kann dein filesystem keine groß-kleinschreibung?

Application::Application()
: menu(makeMenu()), running(true)
{
...
Application::~Application()
{
  delete menu;

sollte aus symmetriegründen nicht zu nem makeMenu ein destroyMenu existieren. auch, wenn es nur delete aufruft.

void invalidPosition() { do_invalidPosition(); }

kann darin keinen sinn ergründen. warum die do_-sachen?

bool Field::equal(Type a, Type b, Type c)
{
  if(a==b && b==c)
    return a!=EMPTY;
  return false;
}

völlig unverständlich. der name equal widerspricht einfach dem inhalt.
Type ist nirgends definiert. suche vergleblich die Type.h. nichtmal ne type.h gibts. auch keine type.hpp oder Type.hpp. grep...
auch "class Type" nicht. und "struct Type" nicht. auch "typedef Type" nicht. hab schon gar keine lust mehr.
haha, es steht in der "field.hpp".

enum Type
{
  PLAYER_1,
  PLAYER_2,
  EMPTY
};

ach, scheu dich einfach nicht, für 5 zeilen ne datei zu bauen. pro globalem typ ne datei. und enum oder class ist ja austauschbar. also kannst später class Type draus machen und alles compilier weiterhin gut durch.

hmm.
bei

Field::Field()
{
  for(int i=0; i<3; ++i)
  {
    for(int j=0; j<3; ++j)
    {
      field[i][j]=EMPTY;
    }
  }
}

frage ich mich, ob Type nicht doch ein struct/class zu sein gewesen hätte mit nem default-ctor.
nee, aber so schleifen dürfen trotzdem weg. mit ner klasse Vector2D mit nem guten konstruktor?

bool Field::full() const

inkonsistent. zu getElement auch isFull.

Type Field::winner() const
{
  for(int i=0; i<3; ++i)
  {
    if(equal(field[i][0], field[i][1], field[i][2]))
      return field[i][0];
  }

  for(int j=0; j<3; ++j)
  {
    if(equal(field[0][j], field[1][j], field[2][j]))
      return field[0][j];
  }

  if(equal(field[0][0], field[1][1], field[2][2]))
    return field[0][0];

  if(equal(field[2][0], field[1][1], field[0][2]))
    return field[0][2];

  return EMPTY;
}

also für if(equal(field[i][0], field[i][1], field[i][2])) wurde equal so modifiziert, daß die bedeutung von isWinning hat.
oh, Field::winner kann EMPTY zurückgeben.
na, dann drängt sich doch fast

Type winner(int x,int y,int dx,int dy)
{
   Type winner=field[x][y];
   if(winner==EMPTY) return EMPTY;
   x+=dx;y+=dy;
   if(field[x][y]!=winner) return EMPTY;
   x+=dx;y+=dy;
   if(field[x][y]!=winner) return EMPTY;
   return winner;
}

an.

aber ich persönlich hätte eh kein 2d-feld genommen, sondern 1d mit den indizes wie rechts neben dir auf dem ziffernblock der tastatur.

Type winner(int pos,int d) const
{
   Type winner=field[pos];
   if(field[pos]==EMPTY) return EMPTY;
   pos+=d;
   if(field[pos]!=winner) return empty;
   pos+=d;
   if(field[pos]!=winner) return empty;
   return winner;
}

auch überlegenswert ist immer ne kleine tabelle statt code.

Type winner() const
{
   static int lines={
      7,8,9,
      4,5,6,
      1,2,3,
      7,4,1,
      8,5,2,
      9,6,3,
      7,5,2,
      9,5,1,}
   ...
}

nee, auch nicht fein.
aber vielleicht

Type winner() const
{
   static int lines={
      1,3, 2,3, 3,3,//nach oben
      1,1, 4,1, 7,1,//nach rechts
      1,4,//nach rechts-oben
      3,2,//nach linksoben
   }
   for(int i=0;i<LENGTHOF(lines);i+=2)
      if(Type winner=winner(lines[i],lines[i+1])
         return winner;
   return EMPTY;
}

ups, hatte eben schon das gefühl, daß EMPTY der wert 0 haben muß. dachte an die performance beim initialisieren von feldern. hier sieht mans auch wieder.

void swapState(int& i)
{
  if(i==0)
    i=1;
  else
    i=0;
}

ach, ach, ach.
ein referenzparameter. den erklärste mir aber mal.
und abgesehen davon heißts besser, damit der funktionsaufruf zu weniger assemblerbefehlen zusammenfällt:

int swapState(int i)
{
   return 1-i;
   //alternativ return i^1;
   //alternativ return i^(1^0);
}

catch(...)
  {
    delete player[0];
    delete player[1];
    throw;
  }

gibts nicht. wer das macht, hat RAII verpaßt.

while(1)

gibts auch nicht. heißt while(true) wenn du pascaller bist oder als c++-er for(;;). while(1) könnte ein basic-hase machen, der zu lange c machte.

std::srand(static_cast<unsigned>(std::time(0)));

was passiert, wenn hier kein static cast steht? ne warnung?

Position HumanPlayer::do_action(Field const& field)
{
  while(1)
  {
    Position pos=engine->getHumanAction();
    if(field.getElement(pos) == EMPTY)
    {
      return pos;
    }
    engine->invalidPosition();
  }
}

nicht erkennbar, was engine->invalidPosition(); machen soll. muß mal lesen, was die funktion macht. ah, sie meldet der engine, daß der zug ungültig war.
der zug. also move. und position ist in spielerkreisen ne stellung.

Move HumanPlayer::getMove(Position const& position)
{
  for(;;)
  {
    Move move=engine->getHumanMove();
    if(isValidMove(field,move))
    {
      return pos;
    }
    engine->rejectHomanMove(move);//manche engines merken sich selber den 
       //letzten zug nicht
  }
}

void ConsoleEngine::do_winner(Type elem)
{
  if(elem == PLAYER_1)
    std::cout<<"Player 1 wins\n";
  else if(elem == PLAYER_2)
    std::cout<<"Player 2 wins\n";
  else std::cout<<"Draw\n";
}

ein array mit texten?

Position ConsoleEngine::do_getHumanAction()
{
  while(1)
  {
    std::cout<<"use numpad for input - where do you want to set? ";
    int num;
    if(std::cin>>num)
    {
      if(0<num && num<=9)
      {
        switch(num)
        {
        case 1:
          return Position(2,0);
        case 2:
          return Position(2,1);
        case 3:
          return Position(2,2);
        case 4:
          return Position(1,0);
        case 5:
          return Position(1,1);
        case 6:
          return Position(1,2);
        case 7:
          return Position(0,0);
        case 8:
          return Position(0,1);
        case 9:
          return Position(0,2);
        }
      }
    }
    std::cout<<"invalid input\n";
  }
}

Position ConsoleEngine::do_getHumanAction()
{
  for(;;)
  {
    cout<<"use numpad for input - where do you want to set? ";
    int num;
    if(std::cin>>num)
       if(range(0,num,9))
          return Position(2-(num-1)%3,(num-1)/3);
   }
}

Type enemy=(search==PLAYER_1 ? PLAYER_2 : PLAYER_1);

?: mag ich net.

Type enemy=search^(PLAYER_1^PLAYER_2);

Position()
  : x(-1), y(-1)
  {}

wozu der default-ctor? muß sowas echt in ein array rein? und warum auf -1 setzen und nicht uninitialized lassen?

und du darfst ruhig in deiner *.cpp using namespace std; schreiben. das zeugt nicht von schlechtem stil.

[kxp]

Hi Shade,

soll das Ganze ein Beispiel für schlechtes Programm-Design sein?

Wenn ja, nicht schlecht geworden.

gruß kxp

[volkard]

kxp schrieb:

Hi Shade,
soll das Ganze ein Beispiel für schlechtes Programm-Design sein?
Wenn ja, nicht schlecht geworden.

lol. gut gebrüllt, löwe.
aber wirkungslos. das overengeneering ist klar. und es werden viele sachen gezeigt, die bei großen projekten lebensnotwendig sind. den code sollten ruhig einige leute mal lesen.

[Gast]

?: mag ich net.

Ich schon.
Allerdings bevorzuge ich, die einzelnen Teile nochmal mit Klammern zu trennen:

// Übrigens, ohne '?' könntest du hier kein const verwenden.
const int x  =  (bla == blubb)  ?  (17) : (23);

[volkard]

Optimizer schrieb:

?: mag ich net.

Ich schon.
Allerdings bevorzuge ich, die einzelnen Teile nochmal mit Klammern zu trennen:

// Übrigens, ohne '?' könntest du hier kein const verwenden.
const int x  =  (bla == blubb)  ?  (17) : (23);

const int x  =  (bla==blubb)["\21\27"];

[Gast]

Hö? Kenn ich gar nicht. Wo ist der Vorteil gegenüber dem '?' ?

[junix]

Irgendwie scheint mir das ganze Projekt - nach dem ersten Überfliegen - reichlich knapp dokumentiert?

-junix

[Christoph]

volkard schrieb:

const int x  =  (bla==blubb)["\21\27"];