Codequalität von meinem TicTacToe

06/06/2010 23:05 Medix#1
Hier mein kleines TicTacToe das ich gestern Nacht entworfen habe.
Es wäre eine Frechheit die Computerzüge als KI zu bezeichnen aber man hat wenigstens einen Gegenspieler. (Eine halbwegs intelligente KI wollt ich demnächst noch entwerfen)
Was für TicTacToe ja eigentlich gut machbar sein sollte.

Es geht mir um meinen Code ich bin kein erfahrener programmiere. Ich öffne halt ab und zu meine IDE und schreib n paar Zeilen.
Daher wollt ich fragen wie eigentlich die qualität vom code ist. Funktionieren tuts ja.


Der Code sollte c++ darstellen, hab nich soviel ahnung vom programmieren ^.^

Ausführbare .exe findet sich im Anhang
Code:
#include <iostream>
#include <windows.h>


using namespace std;

char line1[4] = "XXX";
char line2[4] = "XXX";
char line3[4] = "XXX";


void display()
{
    cout<<"----------------\n";
    cout<<"    ";
    cout<<line1<<endl;
    cout<<"    ";
    cout<<line2<<endl;
    cout<<"    ";
    cout<<line3<<endl;
    cout<<"----------------";
    cout<<"\n";
}

bool checkwin(char token)
{


    if (line1[0] == token && line1[1] == token  && line1[2] == token)
    {
        return 0;


    }
    if (line2[0] == token  && line2[1] == token  && line2[2] == token  )
    {
        return 0;


    }
    if (line3[0] == token  && line3[1] == token  && line3[2] == token  )
    {

        return 0;
    }

    if (line1[0] == token  && line2[0] == token  && line3[0] == token )
    {

        return 0;

    }

    if (line1[0] == token  && line2[1] == token  && line3[2] == token  )
    {
        return 0;


    }


    if (line1[2] == token  && line2[1] == token  && line3[0] == token  )
    {
        return 0;


    }

    if (line1[1] == token && line2[1] == token && line3[1] == token  )
    {
        return 0;

    }


    if (line1[2] == token  && line2[2] == token  && line3[2] == token  )
    {
        return 0;


    }



    else
    {
        return 1;
    }

}


void pcMoveEasy()
{
    for (int j=0; j<4;j++)
    {
        if (line1[j] != 1 && line1[j] !=2)
        {
            line1[j] = 2;
            break;

        }
        if (line2[j] != 1 && line2[j] != 2)
        {
            line2[j] = 2;
            break;

        }
        if (line3[j] != 1 && line3[j] != 2)
        {
            line3[j] = 2;
            break;

        }


    }
}



void playerMove()
{
    int token;
    cin>>token;
    cout<<"\n";

    switch (token)
    {


    case 0:
        break;

    case 1:


        if (line1[0] != 2)
        {
            line1[0] = 1;
        }
        break;

    case 2:

        if (line1[1] != 2)
        {
            line1[1] = 1;
        }
        break;
    case 3:

        if (line1[2] != 2)
        {
            line1[2] = 1;
        }
        break;
    case 4:

        if (line2[0] != 2)
        {
            line2[0] = 1;
        }
        break;
    case 5:

        if (line2[1] != 2)
        {
            line2[1] = 1;
        }
        break;
    case 6:

        if (line2[2] != 2)
        {
            line2[2] = 1;
        }
        break;
    case 7:

        if (line3[0] != 2)
        {
            line3[0] = 1;
        }

        break;
    case 8:

        if (line3[1] != 2)
        {
            line3[1] = 1;
        }
        break;

    case 9:

        if (line3[2] != 2)
        {
            line3[2] = 1;
        }
        break;

    default:

        cout<<"Ungueltige Eingabe! Eingabe muss zwischen 1 und 9 sein!\n";
        break;

    }

}


int main ()
{
    SetConsoleTitle("TicTacToe");
    cout<<"TicTacToe\n";
    cout<<"X = unused field\n";
    cout<<"Use 1-9 as Input ; 1 = first field | 9 = last field | 0 = Computer starts!";
    cout<<"\n\n\n";
    int n = 0;
    display();
    while (n<9)
    {

        playerMove();
        cout<<"Your move:\n";
        display();
        if (!checkwin(1))
        {
            cout<<"You Win!";
            break;
        }

        if (!checkwin(2))
        {
            cout<<"You Lose!";
            break;
        }
        pcMoveEasy();
        cout<<"Computer move:\n";
        display();

        if (!checkwin(1))
        {
            cout<<"You Win!";
            break;
        }

        if (!checkwin(2))
        {
            cout<<"You Lose!";
            break;
        }

        n++;
    }

    system("PAUSE");
    return 0;
}
06/06/2010 23:26 nkkk#2
naja mir gefällt er so mittel kommt eben drauf an was man will,
der Nachteil an den vielen if ist eben, dass wenn man das spiel leicht verändern will(z.B. auf einem 4*4 feld spielen) muss man noch n paar mehr einbauen, deshalb fände ich iwas mit schleifen besser, aber naja.
06/06/2010 23:32 Adroxxx#3
bool checkwin;
Du solltest redundanten Code vermeiden.
Anstatt 10 mal if ( ....) zu schreiben, würde ich eine for - schleife machen und über das array iterieren.

Außerdem würde ich mir eine Einheitliche Notation für die Methoden überlegen.

Einmal schreibst du checkwin(), dann pcMoveEasy().
Würde mir dann schon eine Sache angewöhnen. Entweder checkWin, oder pcmoveeasy.
Wobei ersteres natürlich leserlicher ist. checkWin, pcMoveEasy etc.

Außerdem solltest du nur so selten wie möglich break benutzten.
Klar beim switch natürlich. Aber sonst gibt es nur sehr wenige Fälle wo ein rausbreaken aus einer Schleife die einzige Lösung ist.
Wenn du etwas überlegst wirst du auch eine Möglichkeit finden ohne Break das Programm zu schreiben.
Das benutzten von Break und Goto ist keine schöne Art zu programmieren.
Ausnahme ist wie gesagt switch.

Sprechende Variablennamen.

int n = 0 ;
Wofür soll da n stehen?

Bei der for schleife ist ein int i , j , k ok. Da es halt ein Index ist.
Aber an anderen Stellen solltest du versuchen Verständliche Variablen zu benutzen.

Eine Sprache.
Du solltest dich für eine Sprache entscheiden. Entweder Deutsch oder Englisch.
Nicht beides Mischen.

PHP Code:
cout<<"Ungueltige Eingabe! Eingabe muss zwischen 1 und 9 sein!\n"
PHP Code:
 cout<<"Computer move:\n"
PHP Code:
cout<<"You Lose!"
Naja und keinerlei Kommentare.
Du solltest schon dein Zeugs kommentieren.
Ob es für dich ist, damit du weißt was du da verzapft hast oder für andere, falls du den Source public stellst.
06/06/2010 23:37 Medix#4
Vielen Dank für die Kritik. Garnicht gemerkt ,dass ich Deutsch/english gemischt habe. War wohl etwas spät und konnte mich wohl nicht richtig entscheiden ^.^.

int n = 0; war die zählvariable für die whileschleife.

Aufjedenfall danke hast mir ein paar gute Anregungen gegeben.
06/06/2010 23:39 Adroxxx#5
Quote:
Originally Posted by Medix View Post
Vielen Dank für die Kritik. Garnicht gemerkt ,dass ich Deutsch/english gemischt habe. War wohl etwas spät und konnte mich wohl nicht richtig entscheiden ^.^.

int n = 0; war die zählvariable für die whileschleife.

Aufjedenfall danke hast mir ein paar gute Anregungen gegeben.
Ja, dass n die Zählvariable für die Whileschleife ist, ist mir wohl bewusst. Aber was zählt die While Schleife?

Wenn jemand sich den Code schnell anguckt, weiß er garnciht was das ist.
Sind das die Runden? Anzahl der Felder etc.

Darum sollte man die Variable schon so benennen, dass man erkennt wozu die da ist.
06/06/2010 23:42 Medix#6
Gut werd da in Zukunft dran denken.

Wollt das ding gestern Nacht nur zum laufen bringen daher eher wenig auf passende Variablennamen bzw Kommentare geachtet.
06/07/2010 00:04 Adroxxx#7
Jo und am nächsten Tag wachste auf, bist raus aus deinen Gedankengängen und steht vor 2000 Zeilen unkommentierten code mit variablen wie foo, foobar, tmp, temp, i , a , fu, bla, bla2, blub.

Und dann ist die scheiße am dampfen :D:D

Ja ich spreche aus Erfahrung xD
06/07/2010 00:23 MrSm!th#8
Quote:
Originally Posted by Adroxxx View Post
Jo und am nächsten Tag wachste auf, bist raus aus deinen Gedankengängen und steht vor 2000 Zeilen unkommentierten code mit variablen wie foo, foobar, tmp, temp, i , a , fu, bla, bla2, blub.

Und dann ist die scheiße am dampfen :D:D

Ja ich spreche aus Erfahrung xD
hey, nichts gegen tmp und temp, das sind meine lieblingsvariablennamen :awesome:

Ich möchte noch ein wenige ergänzen:

using namespace ist nicht gerade schön.
Normalerweise schreibt man die Bereichsauflöser immer mit, also std::cout und macht keine using namespace Direktive.
Klar, in diesem kleinen Projekt ist das noch kein Problem, aber man sollte es sich am besten direkt richtig angewöhnen.

Desweiteren hast du doch das Glück, eine teilweise objektorientierte Sprache zu verwenden.
Nutze doch die Möglichkeiten von Klassen ;)
Das Zauberwort ist Datenkapselung, dann lässt sich, wie schon erwähnt wurde, später besser was ergänzen.

Ach ja, als Ergänzung zu Adroxxxs Kommentar:

Es hat sich so eingebürgert, dass man ein Kürzel des Typs einer Variable am Anfang des Namens hat (klein)

also
Code:
char* szSomeString //sz = string-zero-terminated wahlweise auch:
char* pszSomeString //p für pointer
int nCount //n kommt aus der Mathematik, für ganze Zahlen
bool bWin //b für bool
//usw...
06/07/2010 15:28 nkkk#9
Quote:
Originally Posted by Adroxxx View Post
Außerdem solltest du nur so selten wie möglich break benutzten.
wiso, was kann man den sonst nehmen was besser ist?
06/08/2010 02:38 Bot_interesierter#10
@MrSm!th
System Hungarian ist Blödsinn in C++, das macht nur Sinn in untypisierten Programmiersprachen, Sinnvoller ist eine Notation die eine Aussage über die Eigenschaften der Variablen zulässt, zb ein _ für locale Variablen m_ für member, g_ für globals(die man vermeiden sollte) und dann sollte natürlich der restliche Namen einen Sinn ergeben.

Ungarische Notation ? Wikipedia
kannst dir auch mal dazu durchlesen, allerdings bin ich insgesamt kein Freund der Ungarischen Notation, sie ist zu aufwendig und man kann schon durch kleine kniffe die Lesbarkeit des Codes erhöhen, ansonsten ist es Sinnvoll die Notation an die Bibliotheken mit denen man arbeitet anzupassen.
06/08/2010 08:21 Adroxxx#11
Quote:
Originally Posted by nkkk View Post
wiso, was kann man den sonst nehmen was besser ist?
Besser so Programmieren, dass man keine Sprunganweisung braucht.

Quote:
Originally Posted by Bot_interesierter View Post
@MrSm!th
System Hungarian ist Blödsinn in C++, das macht nur Sinn in untypisierten Programmiersprachen, Sinnvoller ist eine Notation die eine Aussage über die Eigenschaften der Variablen zulässt, zb ein _ für locale Variablen m_ für member, g_ für globals(die man vermeiden sollte) und dann sollte natürlich der restliche Namen einen Sinn ergeben.

Ungarische Notation ? Wikipedia
kannst dir auch mal dazu durchlesen, allerdings bin ich insgesamt kein Freund der Ungarischen Notation, sie ist zu aufwendig und man kann schon durch kleine kniffe die Lesbarkeit des Codes erhöhen, ansonsten ist es Sinnvoll die Notation an die Bibliotheken mit denen man arbeitet anzupassen.
Joa hatte mir die UN mal beim MFC Programmieren angewöhnt. Mittlerweile beim Java Programmieren aber auch schon wieder verworfen, da es da nutzlos ist.
06/08/2010 14:00 MrSm!th#12
Quote:
Originally Posted by Bot_interesierter View Post
@MrSm!th
System Hungarian ist Blödsinn in C++, das macht nur Sinn in untypisierten Programmiersprachen, Sinnvoller ist eine Notation die eine Aussage über die Eigenschaften der Variablen zulässt, zb ein _ für locale Variablen m_ für member, g_ für globals(die man vermeiden sollte) und dann sollte natürlich der restliche Namen einen Sinn ergeben.

Ungarische Notation ? Wikipedia
kannst dir auch mal dazu durchlesen, allerdings bin ich insgesamt kein Freund der Ungarischen Notation, sie ist zu aufwendig und man kann schon durch kleine kniffe die Lesbarkeit des Codes erhöhen, ansonsten ist es Sinnvoll die Notation an die Bibliotheken mit denen man arbeitet anzupassen.
Wow, danke, das wusste ich noch gar nicht.
mal wieder typisch MS...

Dann werde ich mal versuchen, mir abzugewöhnen, was ich mir erst mühsam angewöhnen "musste" -.-"

Mir ist klar, dass die Eigenschaften auch gezeigt werden sollen, deshalb kenne ich zb. auch m_pszSomeSting!
Aber gut, das mit den Datentyppräfixen werde ich mir versuchen wieder ab zu gewöhnen, sollte ja nicht ganz so schwer sein :D
Btw das _ für _local kannte ich auch noch nicht, danke.
06/09/2010 11:00 scbiz#13
Die Beiträge, die sich mit der Performance eines TicTacToe-Spiels beschäftigen, ignoriere ich einfach mal gekonnt. :facepalm:

Quote:
Originally Posted by Adroxxx View Post
Naja und keinerlei Kommentare.
Du solltest schon dein Zeugs kommentieren.
Bring ihn bloß nicht auf diese Schiene. :mad:

Quote:
Originally Posted by Ed Post
  • Real Programmers don't need comments-- the code is obvious.
(Quelle: [Only registered and activated users can see links. Click Here To Register...])

Das ist zwar eine Parodie auf Real Men Don't Eat Quiche und handelt von Pascal / Fortran, doch ist der Punkt auf jede mir bekannte Programmiersprache projizierbar. Okay, bis auf die esoterischen Programmiersprachen, bei denen der Quelltext aus Bildern besteht.
06/09/2010 11:15 comentdenner#14
MrSm!th doppelpost. x]

Darf ich mal fragen was der unterschied zwischen der c++ sprache und die der vb.net sprach eist? Abgesehen davon dass c++ um längen schwieriger ist.
06/09/2010 12:03 scbiz#15
Quote:
Originally Posted by comentdenner View Post
MrSm!th doppelpost. x]

Darf ich mal fragen was der unterschied zwischen der c++ sprache und die der vb.net sprach eist? Abgesehen davon dass c++ um längen schwieriger ist.
Toller Äpfel-Birnen-Vergleich :)