Preview: Mein Tutorial::TicTacToe

[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"];

[volkard]

Optimizer schrieb:

Hö? Kenn ich gar nicht.

der arrayzugriff auf ein stringliteral ist ja auch mit der weisheit
a[b]==*(a+b) und a+b==b+a folglich a[b]==b[a] obfuscated.

Optimizer schrieb:

Wo ist der Vorteil gegenüber dem '?' ?

beides schlecht, ich hab nur "Übrigens, ohne '?' könntest du hier kein const verwenden" widerlegt.

[volkard]

junix schrieb:

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

macht gar nix. guter code braucht keine kommentare.

[ThAlb]

volkard schrieb:

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

Warum so kompliziert? Wie wärs mit

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

Für viele dürfte das noch eher verständlich sein...

[Shade Of Mine]

volkard schrieb:

als der erste bildschirm voll ist schonmal kommentar...

Jo - aber was soll ich sonst mit der LGPL Lizenz machen?

kann dein filesystem keine groß-kleinschreibung?

Was verwendest du denn?
Stimmt - Name der Header == Name des Typen - klingt vernünftig

Wie sieht es mit Dateien ohne Klassendefinition aus, zB main.cpp?

void invalidPosition() { do_invalidPosition(); }

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

Ich dachte ich Overengineere mit dem Template Method Pattern um uU noch ein Logging oder ähnliches.

Ich dachte mir eigentlich - kann ja nix Schaden... Oder schadet es doch?

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.

ich mach immer while(1)

?: mag ich net.

Type enemy=search^(PLAYER_1^PLAYER_2);

Was spricht gegen ?: in diesem Fall?
ein

Type enemy;
if(search==PLAYER_1)
  enemy=PLAYER_2;
...

Kann ja nicht die Lösung sein, oder?

Optimizer schrieb:

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

Natürlich. Darauf ist das Design ja ausgelegt.
Allerdings muss man dann das Menü noch etwas anpassen...

@junix:
Ich habe überlegt ob ich es sehr ausführlich dokumentieren sollte, damit es Anfänger leichter verstehen. Allerdings bin ich zu dem Schluss gekommen: sie sollen es lieber selber 'rausfinden' und sehen wie gut lesbarer Code keine unnötigen Kommentare braucht.

Deshalb will ich ja auch, dass ihr mir alle meine Fehler aufzeigt

[volkard]

Shade Of Mine schrieb:

volkard schrieb:

als der erste bildschirm voll ist schonmal kommentar...

Jo - aber was soll ich sonst mit der LGPL Lizenz machen?

hast doch eh nicht die ganze lizenz drin im header.
aus
* AUTHOR: Toni Schornboeck toni@schornboeck.net
* PROJECT: TicTacToe
* (c) 2004 by Toni Schornboeck
würde ich machen
(c) 2004 by Toni Schornboeck toni@schornboeck.net

ist dieser passus notwendig?
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
wenns doch eh in der LGPL nochmal steht, daß da keine garantie drauf ist?

ich mach meine files einfach mit (c) zu. wenn die dann in falsche hände gerät,
isse halt unverwertbar. dann reicht doch im projet eine datei, die das
projekt mit allen files in die LGPL stellt, oder?

ein copyleft-vermerk würde es beim tic-tac-toe aber auch tun.

kann dein filesystem keine groß-kleinschreibung?

Was verwendest du denn?

WinXP, das kann groß-kleinschreibung. ich mach dateinamen wie die klassennamen, seit es geht, also bei mir seit dem umstieg von dos auf win95.

Stimmt - Name der Header == Name des Typen - klingt vernünftig
Wie sieht es mit Dateien ohne Klassendefinition aus, zB main.cpp?

die bleiben klein. klassen schreib ich groß und funktionen klein. so sehe ich, daß in der datei BitField.cpp die implemetierung einer klassse steht, und in der bitwise.h nur funktionen stehen.

Ich dachte ich Overengineere mit dem Template Method Pattern um uU noch ein Logging oder ähnliches.
Ich dachte mir eigentlich - kann ja nix Schaden... Oder schadet es doch?

macht mehr tipparbeit und die ausführbare langsamer und größer. wenn ich recht sah, hat das pattern hier gar keinen nutzen, dann isses störend. wenns ne stelle gibt, wo es was feines macht, isses ein lehrreiches beispiel.

?: mag ich net.

Type enemy=search^(PLAYER_1^PLAYER_2);

Was spricht gegen ?: in diesem Fall?
ein

Type enemy;
if(search==PLAYER_1)
  enemy=PLAYER_2;
...

Kann ja nicht die Lösung sein, oder?

nein? aber doch! in ne funktion namens Type gegenspieler(Type spieler) rein und es ist gut.
steht im schwierigen code rstmal das einfache
Type enemy=gegenspieler(search);
, dann kannste dir immernoch überlegen, ob du innerhalb von gegenspieler sündigst. ich rate dort aber streng von ?: ab, wegen nutzlosigkeit. kann es sein, daß iht ?:-verfechter einfach zu faul seid, auch einzeiler in ne funktion mit sprechendem namen auszulagern?
bei meinem vorschlag

Type enemy=search^(PLAYER_1^PLAYER_2);

waren die klammern absicht. es war nicht gedacht als magischen hack, der viel fachwissen um interne bitsetzungen weiß, sondern um ein programmiermuster, wo man ein paar allgemeine eigenschaften von ^ ausnutzt.
man kanns auch lesen als

const int UMSCHALTER_ZWISCHEN_PLAYER_1_UND_PLAYER_2=PLAYER_1^PLAYER_2;
Type enemy=search^UMSCHALTER_ZWISCHEN_PLAYER_1_UND_PLAYER_2;