Tell me what i did wrong!

This is a discussion on Tell me what i did wrong! within the C++ Programming forums, part of the General Programming Boards category; It does run, but i would like some feedback on how i could have made it better; For those who ...

  1. #1
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100

    Post Tell me what i did wrong!

    It does run, but i would like some feedback on how i could have made it better; For those who aren't sure the values you see repeated, 88 and 79, are ASCII X and O respectively.

    I'm not so interested in portability issues, more functionality issues that would make it smoother overall!

    Remember, this is my real program in 3 years!

    Code:
    #include <cstdlib>
    #include <iostream>
    #include <windows.h>
    #include <stdio.h>
    #include <conio.h>
    
    using namespace std;
    
    void gotoxy(int, int); // Function that places cursor for DOS
    const void drawborder(); // Draws the border around the edge
    const void spalsh(); // Draws the 'splash screen'
    inline const int whoFirst();
    void whoWon(int);
    enum player {ONE, TWO};
    
    class Board  //The class for the board
    {
        public:
            Board() ;
            ~Board() {} ;
            void drawBoard() const;
            int compMove(); //the computers move!
            bool checkBoard(); //check if the game is won!
            void makeMove(int move, int player); // make the move player
            bool testMove(int move) const;
            void testBoard() { theBoard[1][1] = 88;}; //dec. 88 is ASCII X
            bool isWon() { return (won); };
        private:
            int theBoard[3][3];
            bool won;
    };
    Board::Board(){ // Constructor, initalize theBoard with the asii number 49 - 57 for 1-9
    
        int in=49;
        for(int x=0; x<3; x++){
            for(int y=0;y<3;y++){
                    theBoard[y][x] = in++;
    
                }
            }
            won = false;
    
    
        }
    int Board::compMove() {
            if(theBoard[1][0] == 79 && theBoard[2][0]==79 && theBoard[0][0]==49 || theBoard[0][1]==79 && theBoard[0][2]==79 && theBoard[0][0]==49 || theBoard[0][0]==49 && theBoard[1][1] == 79 && theBoard[2][2]==79){
                return 1;
            }
            if(theBoard[0][0] == 79 && theBoard[2][0]==79 && theBoard[1][0]==50 || theBoard[1][1]==79 && theBoard[1][2]==79 && theBoard[1][0]==50) {
                return 2;
            }
            if(theBoard[1][0] == 79 && theBoard[2][0]==51 && theBoard[0][0]==79 || theBoard[2][2]==79 && theBoard[2][0]==51 && theBoard[2][1]==79 || theBoard[2][0]==51 && theBoard[1][1] == 79 && theBoard[0][2]==79){
                return 3;
            }
            if(theBoard[0][0] == 79 && theBoard[0][1]==52 && theBoard[0][2]==79 || theBoard[0][1]==52 && theBoard[1][1]==79 && theBoard[2][1]==79) {
                return 4;
            }
            if(theBoard[0][1] == 79 && theBoard[1][1]==53 && theBoard[2][1]==79 || theBoard[1][0]==79 && theBoard[1][1]==53 && theBoard[1][2]==79) {
                return 5;
            }
            if(theBoard[0][1] == 79 && theBoard[1][1]==79 && theBoard[2][1]==54 || theBoard[2][0]==79 && theBoard[2][2]==79 && theBoard[2][1]==54) {
                return 6;
            }
            if(theBoard[0][1] == 79 && theBoard[0][0]==79 && theBoard[0][2]==55 || theBoard[1][2]==79 && theBoard[2][2]==79 && theBoard[0][2]==55 || theBoard[0][2]==55 && theBoard[1][1] == 79 && theBoard[2][0]==79){
                return 7;
            }
            if(theBoard[1][1] == 79 && theBoard[1][0]==79 && theBoard[1][2]==56 || theBoard[0][2]==79 && theBoard[1][2]==56 && theBoard[2][2]==79) {
                return 8;
            }
            if(theBoard[0][2] == 79 && theBoard[1][2]==79 && theBoard[2][2]==57 || theBoard[2][1]==79 && theBoard[2][2]==57 && theBoard[2][0]==79 || theBoard[2][2]==57 && theBoard[1][1] == 79 && theBoard[0][0]==79){
                return 9;
            }
    
            if(theBoard[1][0] == 88 && theBoard[2][0]==88 && theBoard[0][0]==49 || theBoard[0][1]==88 && theBoard[0][2]==88 && theBoard[0][0]==49 || theBoard[0][0]==49 && theBoard[1][1] == 88 && theBoard[2][2]==88){
                return 1;
            }
            if(theBoard[0][0] == 88 && theBoard[2][0]==88 && theBoard[1][0]==50 || theBoard[1][1]==88 && theBoard[1][2]==88 && theBoard[1][0]==50) {
                return 2;
            }
            if(theBoard[1][0] == 88 && theBoard[2][0]==51 && theBoard[0][0]==88 || theBoard[2][2]==88 && theBoard[2][0]==51 && theBoard[2][1]==88 || theBoard[2][0]==51 && theBoard[1][1] == 88 && theBoard[0][2]==88){
                return 3;
            }
            if(theBoard[0][0] == 88 && theBoard[0][1]==52 && theBoard[0][2]==88 || theBoard[0][1]==52 && theBoard[1][1]==88 && theBoard[2][1]==88) {
                return 4;
            }
            if(theBoard[0][1] == 88 && theBoard[1][1]==53 && theBoard[2][1]==88 || theBoard[1][0]==88 && theBoard[1][1]==53 && theBoard[1][2]==88) {
                return 5;
            }
            if(theBoard[0][1] == 88 && theBoard[1][1]==88 && theBoard[2][1]==54 || theBoard[2][0]==88 && theBoard[2][2]==88 && theBoard[2][1]==54) {
                return 6;
            }
            if(theBoard[0][1] == 88 && theBoard[0][0]==88 && theBoard[0][2]==55 || theBoard[1][2]==88 && theBoard[2][2]==88 && theBoard[0][2]==55 || theBoard[0][2]==55 && theBoard[1][1] == 88 && theBoard[2][0]==88){
                return 7;
            }
            if(theBoard[1][1] == 88 && theBoard[1][0]==88 && theBoard[1][2]==56 || theBoard[0][2]==88 && theBoard[1][2]==56 && theBoard[2][2]==88) {
                return 8;
            }
            if(theBoard[0][2] == 88 && theBoard[1][2]==88 && theBoard[2][2]==57 || theBoard[2][1]==88 && theBoard[2][2]==57 && theBoard[2][0]==88 || theBoard[2][2]==57 && theBoard[1][1] == 88 && theBoard[0][0]==88){
                return 9;
            }
            if(theBoard[1][1]==53){
                return 5;
            }
            if(theBoard[0][0]==88 && theBoard[1][2]==56 && theBoard[1][0]!=88){
                return 8;
            }
            if(theBoard[2][0]==88 && theBoard[1][2]==56 && theBoard[1][0]!=88){
                return 8;
            }
            if(theBoard[0][2]==88 && theBoard[1][0]==50 && theBoard[1][2]!=88){
                return 2;
            }
            if(theBoard[2][2]==88 && theBoard[1][0]==50 && theBoard[1][2]!=88){
                return 2;
            }
            if(theBoard[1][0]==88 && theBoard[2][0]==55 && theBoard[0][2]!=88){
                return 3;
            }
            if(theBoard[2][1]==88 && theBoard[2][2]==57 && theBoard[0][0]!=88){
                return 9;
            }
            if(theBoard[1][2]==88 && theBoard[0][2]==55 && theBoard[2][0]!=88){
                return 8;
            }
            if(theBoard[0][1]==88 && theBoard[0][0]==49 && theBoard[2][2]!=88){
                return 1;
            }
            if(theBoard[1][1]=88){
                int ranCho = (rand() % (9 - 1 + 9) + 1);
                while (ranCho == 5){
                    ranCho=(rand() % (9 - 1 + 9) + 1);
                    (rand() % (9 - 1 + 9) + 1);
                }
                return ranCho;
            }
    
    }
    
    bool Board::checkBoard(){
            if((theBoard[0][0] == 88 && theBoard[1][0] ==88 && theBoard[2][0] ==88) || (theBoard[0][0] ==79 && theBoard[1][0]==79 && theBoard[2][0]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[0][1] == 88 && theBoard[1][1] ==88 && theBoard[2][1] ==88) || (theBoard[0][1] ==79 && theBoard[1][1]==79 && theBoard[2][1]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[0][2] == 88 && theBoard[1][2] ==88 && theBoard[2][2] ==88) || (theBoard[0][2] ==79 && theBoard[1][2]==79 && theBoard[2][2]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[0][0] == 88 && theBoard[0][1] ==88 && theBoard[0][2] ==88) || (theBoard[0][0] ==79 && theBoard[0][1]==79 && theBoard[0][2]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[1][0] == 88 && theBoard[1][1] ==88 && theBoard[1][2] ==88) || (theBoard[1][0] ==79 && theBoard[1][1]==79 && theBoard[1][2]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[2][0] == 88 && theBoard[2][1] ==88 && theBoard[2][2] ==88) || (theBoard[2][0] ==79 && theBoard[2][1]==79 && theBoard[2][2]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[0][0] == 88 && theBoard[1][1] ==88 && theBoard[2][2] ==88) || (theBoard[0][0] ==79 && theBoard[1][1]==79 && theBoard[2][2]==79)) {
                this->won = true;
                return true;
            }
            else if((theBoard[0][2] == 88 && theBoard[1][1] ==88 && theBoard[2][0] ==88) || (theBoard[0][2] ==79 && theBoard[1][1]==79 && theBoard[2][0]==79)) {
                this->won = true;
                return true;
            }
            else {
                return false;
            }
        }
    void Board::drawBoard() const {
    /*   |   |
      ------------
         |   |
      ------------
         |   |
    
         Draw the board*/
    
         gotoxy(50,8);
         cout << char(theBoard[0][0]) << "  | " << char(theBoard[1][0]) << " | " << char(theBoard[2][0]);
         gotoxy(50,9);
         cout << "-----------";
         gotoxy(50,10);
         cout << char(theBoard[0][1]) << "  | " << char(theBoard[1][1]) << " | " << char(theBoard[2][1]);
         gotoxy(50,11);
         cout << "-----------";
         gotoxy(50,12);
         cout << char(theBoard[0][2]) << "  | " << char(theBoard[1][2]) << " | " << char(theBoard[2][2]);
    
    }
    
    void Board::makeMove(int move, int player){
        switch (move){
            case 1:
                if(player==ONE){
                    theBoard[0][0]=88;
                }
                else{
                    theBoard[0][0]=79;
                }
                break;
            case 2:
                if(player==ONE){
                    theBoard[1][0]=88;
                }
                else{
                    theBoard[1][0]=79;
                }
                break;
            case 3:
                if(player==ONE){
                    theBoard[2][0]=88;
                }
                else{
                    theBoard[2][0]=79;
                }
                break;
            case 4:
                if(player==ONE){
                    theBoard[0][1]=88;
                }
                else{
                    theBoard[0][1]=79;
                }
                break;
            case 5:
                if(player==ONE){
                    theBoard[1][1]=88;
                }
                else{
                    theBoard[1][1]=79;
                }
                break;
            case 6:
                if(player==ONE){
                    theBoard[2][1]=88;
                }
                else{
                    theBoard[2][1]=79;
                }
                break;
            case 7:
                if(player==ONE){
                    theBoard[0][2]=88;
                }
                else{
                    theBoard[0][2]=79;
                }
                break;
            case 8:
                if(player==ONE){
    
                    theBoard[1][2]=88;
                }
                else{
                    theBoard[1][2]=79;
                }
                break;
            case 9:
                if(player==ONE){
                    theBoard[2][2]=88;
                }
                else{
                    theBoard[2][2]=79;
                }
                break;
            }
          this->drawBoard();
        }
    bool Board::testMove(int move) const{
                if (move == 1 && theBoard[0][0]!= 49){
                    return false;
                }
                else if(move ==2 && theBoard[1][0]!= 50){
                    return false;
                }
                else if(move ==3 && theBoard[2][0]!= 51){
                    return false;
                }
                else if(move ==4 && theBoard[0][1]!= 52){
                    return false;
                }
                else if(move ==5 && theBoard[1][1]!= 53){
                    return false;
                }
                else if(move ==6 && theBoard[2][1]!= 54){
                    return false;
                }
                else if(move ==7 && theBoard[0][2]!= 55){
                    return false;
                }
                else if(move ==8 && theBoard[1][2]!= 56){
                    return false;
                }
                else if(move ==9 && theBoard[2][2]!= 57){
                    return false;
                }
                else {
                    return true;
                }
            }
    
    
    const void drawborder(){ // Draw Border around edge
                for (int x = 1; x < 79; x++){
            gotoxy(x,0);
            cout <<"-";
            gotoxy(x,23);
            cout <<"-";
        }
    
        for(int y =0; y <= 23; y++){
                gotoxy(0,y);
                cout << "|";
                gotoxy(79,y);
                cout << "|";
                }
            }
    
    void gotoxy(int x, int y) // Place the cursor in DOS
    {
      COORD coord;
      coord.X = x;
      coord.Y = y;
      SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), coord);
    }
    
    const void splash(){
            gotoxy(22,10);
            cout << "Welcome to Solo TicTacToe!";
            int blink=0;
            int ch =0;
            time_t start_time, cur_time;
            int test=0;
    
        do {
    
            switch(blink){
                case 0:
                    gotoxy(22,11);
                    cout << "press any key to continue";
                    blink=1;
                    Sleep(1000);
                    continue;
                case 1:
                    gotoxy(22,11);
                    cout << "                         ";
                    blink=0;
                    Sleep(1000);
                    continue;
                }
    
    
    
    
            }while((ch = kbhit()) == 0);
            gotoxy(22,11);
            cout << "                            ";
            gotoxy(22,10);
            cout << "                            ";
            ch=getch();
        }
    
    inline const int whoFirst(){
        srand(time(NULL));
        return (rand() % (1 - 0 + 1) + 0);
    }
    
    void whoWon(int player){
        gotoxy(4,9);
        cout << "                                 ";
        gotoxy(4,10);
        cout << "                                  ";
        gotoxy(4,11);
        cout << "                                   ";
        if(player==3){
            gotoxy(4,10);
            cout << "Tie!!!!!!!!!!!";
            return;
        }
        if(player==ONE){
            gotoxy(4,10);
            cout << "Computer Player Wins!!!!!!";
            return;
        }
        if(player==TWO){
            gotoxy(4,11);
            cout << "You Win!!!!!!!!!!!!!";
            return;
        }
    }
    
    
    
    int main(int argc, char *argv[])
    {
    
        drawborder();
        splash();
        Board TicTac;
        TicTac.drawBoard();
        int whoTurn = whoFirst(), choice, ch, count=0; // Find out who's going first 0 for player, 1 for comp
        while (!(TicTac.isWon())){
            if(count==9){
                whoTurn=3;
                whoWon(whoTurn);
                break;
            }
    
            if(whoTurn == ONE){
    
                gotoxy(4,9);
                cout << "                 ";
                gotoxy(4,9);
                cout << "Your Turn!";
                gotoxy(4,10);
                cout << "What spot would you like (1-9): ";
                cin >> choice;
    
                    if (choice < 0 || choice >10){ //if the choice is out side 1-9 clear and retry.
                        gotoxy(4,10);
                        cout << "                                          ";
                        continue;
                    }
    
                  if (TicTac.testMove(choice)){ //Pass to testMove and check the move
                          TicTac.makeMove(choice, whoTurn);
    
                          if(TicTac.checkBoard()){
                                break;
                            }
                          whoTurn=TWO;
                          count++;
                          continue;
                        }
                    else{
                        gotoxy(4,10);
                        cout << "                                           ";
                        continue;
                    }
            }
            if(whoTurn == TWO){
                gotoxy(4,9);
                cout << "Computers Turn!";
                gotoxy(4,10);
                cout << "What spot should i take...           ";
                choice = TicTac.compMove();
                gotoxy(4,11);
                cout << "I choose spot " << choice;
                TicTac.makeMove(choice, whoTurn);
                whoTurn=ONE;
                    if(TicTac.checkBoard()){
                            break;
                            }
                Sleep(2000);
                gotoxy(4,11);
                cout << "                             ";
    
                count++;
                continue;
            }
    
        }
        if (whoTurn!=3){
            whoWon(whoTurn);
        }
    
    
    
    
    
        gotoxy(2,22);
        system("PAUSE");
        return EXIT_SUCCESS;
    }

  2. #2
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,893
    For those who aren't sure the values you see repeated, 88 and 79, are ASCII X and O respectively.
    So why did you use the numbers? Isn't 'X' far more clear than 88?
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  3. #3
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    Quote Originally Posted by CornedBee View Post
    So why did you use the numbers? Isn't 'X' far more clear than 88?
    Ahh, yes. Well i couldn't remember how to make a char array. so i used a int array instead!

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    I already mentioned in one of your other threads that you should not use ASCII values, but rather the single-quoted character, e.g. '1' instead of 49, 'X' instead of 88, etc.

    I think your compmove function can be done in a different way, firstly by not having almost identical code for X and O repeated. Make a function that checks for X or O depending on which you want to check.


    Code:
                int ranCho = (rand() &#37; (9 - 1 + 9) + 1);
                while (ranCho == 5){
                    ranCho=(rand() % (9 - 1 + 9) + 1);
                    (rand() % (9 - 1 + 9) + 1);
                }
                return ranCho;
    (Note the red line doesn't actually do anything (other than use up a random number and some time).
    Why not:
    Code:
                int ranCho;
                do {
                    ranCho=(rand() % (9 - 1 + 9) + 1);
                } while(ranCho == 5);
                return ranCho;
    Picking a random line:
    Code:
            if(theBoard[1][0] == 79 && theBoard[2][0]==51 && theBoard[0][0]==79 || 
               theBoard[2][2]==79 && theBoard[2][0]==51 && theBoard[2][1]==79 || 
               theBoard[2][0]==51 && theBoard[1][1] == 79 && theBoard[0][2]==79){
                return 3;
            }
    You are checking the same cell thrice: theBoard[2][0] == 51. You could do:
    Code:
            if((theBoard[1][0] == 'O' && theBoard[0][0]=='O' || 
                 theBoard[2][2]=='O' && theBoard[2][1]=='O'  ||
                 theBoard[1][1] == 'O' && theBoard[0][2]=='O') && theBoard[2][0] == '3') {
                return 3;
            }
    In CheckBoard, you can use loops, and check if there are three of the same marker (X or 0)
    in a row diagonally, vertically or horizontally. Just compare the first in the row with the two next ones, and you know that it's a full row.

    In makeMove:
    Code:
                if(player==ONE){
                    theBoard[0][0]=88;
                }
                else{
                    theBoard[0][0]=79;
                }
    Why not just pass (or figure out) the marker in the beginning of the function, and then set the cell to the right value, e.g.

    Code:
    ...
       char marker;
       if (player == ONE) marker = 'O';
       else marker = 'X';
       ... 
       theBoard[0][0] = marker;
    That way, you don't need 9 if-staments that do the same thing, and repeat the same array indexing twice. Repeating the same code is a bad thing.

    --
    Mats
    Last edited by matsp; 05-23-2008 at 05:53 PM.
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by Terran View Post
    Ahh, yes. Well i couldn't remember how to make a char array. so i used a int array instead!
    You can set an integer to 'X' if you like - the compiler will happily translate it to 88.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    Quote Originally Posted by matsp View Post
    You can set an integer to 'X' if you like - the compiler will happily translate it to 88.

    --
    Mats
    I had forgotten about using the single quotes for char instead of double quotes for char const*.

    Code:
        int ranCho = (rand() % (9 - 1 + 9) + 1);
                while (ranCho == 5){
                    ranCho=(rand() % (9 - 1 + 9) + 1);
                    (rand() % (9 - 1 + 9) + 1);
                }
                return ranCho;
    That actually seemed to sneak in under the radar, it's actually just a typo. :-p.

    How would i go about a diagonally check loop? I understand vertical and horizontal...
    Very good suggestions on the rest!

  7. #7
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    >> I had forgotten about using the single quotes for char instead of double quotes for char const*.
    FYI, people keep mentioning it because they expect you to go ahead and change it now that you know how to do it properly.

  8. #8
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    Yeah, i understand, i just wanted to get the full source done and posted before i went and started revamping the whole thing, i'm going over all the code and trying to make it better.

  9. #9
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,304
    You comitted some terrible programming sins

    Code duplication:
    compMove, checkboard, makeMove, and testMove in particular all contain unacceptable amounts of duplication.

    Magic numbers:
    Usage of the numbers 49 to 57, 88, 79, and even 1 to 9 directly (at the very least), was very naughty. You did it right with ONE and TWO, but you should have used NEITHER instead of using 3 - be consistent.
    Actually if you remove the code duplication by correctly using loops then many of those magic numbers will disappear anyway. Those that don't must be defined as meaningful constants.

    That's all that I feel is worth saying about that code. If it weren't for those two things I'd say it's okay, in fact well done for your first program in a long time! Nothing else is worth changing anyway until you have completely gotten out of those two bad habbits. I know it seems quite hard to do the above right now, but it is absolutely essential to be any kind of successful programmer.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  10. #10
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    yeah i figured that about the loops. Using tons of if statements always makes me feel dirty (in a bad way). I'm not sure how to go about a nested diagonal loop.

  11. #11
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Here's my very quick game - without tons of if's - but far from perfect...

    The main idea is the IterationRule array, describing how each line should be iterated in the board. I also made the board one-dimensional for simplicity and internally all indices are zero-based. Only for user input/output I do +1 or -1.

    Another thing that simplifies it a bit is that empty squares contain '\0', not '1' ... '9'. Square numbers are only printed by the output function. If you wanted to keep the numbers, it might be easier for you to check if a square is free by doing:
    Code:
    if (board[i][j] >= '1' && board[i][j] <= '9') {
        //this square is free
    }
    Now, to get rid of the few if's in play_game, you might make an abstract Player class and derive HumanPlayer and ComputerPlayer from it.

    Because <conio.h> is non-standard, no gotoxy for me, only sequential output.

    Code:
    #include <iostream>
    #include <sstream>
    #include <string>
    #include <ctime>
    #include <cctype>
    
    struct IterationRule
    {
        unsigned start, step, count;
    };
    
    const IterationRule lines[8] = {
        { 0, 1, 3 },
        { 3, 1, 3 },
        { 6, 1, 3 },
        { 0, 3, 3 },
        { 1, 3, 3 },
        { 2, 3, 3 },
        { 0, 4, 3 },
        { 2, 2, 3 }
    };
    
    const unsigned line_count = sizeof(lines) / sizeof(lines[0]);
    
    /*
        iterates arr according to rules and returns the number of occurrences of what
    */
    int count(const char arr[], char what, const IterationRule& rules)
    {
        int counter = 0;
        for (unsigned step_count = 0, pos = rules.start; step_count != rules.count; ++step_count, pos += rules.step) {
            if (arr[pos] == what) {
                ++counter;
            }
        }
        return counter;
    }
    
    /*
        iterates arr according to rules,
        returns the index of the first occurrence of what or -1 if not found
    */
    int find(const char arr[], char what, const IterationRule& rules)
    {
        for (unsigned step_count = 0, pos = rules.start; step_count != rules.count; ++step_count, pos += rules.step) {
            if (arr[pos] == what) {
                return pos;
            }
        }
        return -1;
    }
    
    /*
        returns the index of the position which would get player a full line
        or -1 if no such position
    */
    int win_in_one(const char board[], char player)
    {
        for (unsigned i = 0; i != line_count; ++i) {
            if (count(board, player, lines[i]) == 2) {
                int empty_pos = find(board, '\0', lines[i]);
                if (empty_pos != -1) {
                    return empty_pos;
                }
            }
        }
        return -1;
    }
    
    /*
        computer players logic (very primitive)
    */
    int pick_move(const char board[], char me, char opponent)
    {
        //check for instant winning move
        int move = win_in_one(board, me);
        if (move != -1) {
            return move;
        }
    
        //check for defensive move
        move = win_in_one(board, opponent);
        if (move != -1) {
            return move;
        }
    
        //select random move - this could be improved :)
        do {
            move = rand()&#37;9;
        } while (board[move]);
        return move;
    }
    
    bool has_won(const char board[], char player)
    {
        for (unsigned i = 0; i != line_count; ++i) {
            if (count(board, player, lines[i]) == 3) {
                return true;
            }
        }
        return false;
    }
    
    /*
        draws a single line of the board, starting at index pos
        empty squares are represented by square number
        squares are separated by pipe characters
    */
    void draw_line(const char board[], unsigned pos)
    {
        for (unsigned i = 0, end = pos + 3; pos != end; ++i, ++pos) {
            if (board[pos]) {
                std::cout << board[pos];
            }
            else {
                std::cout << pos + 1;
            }
            if (i < 2) {
                std::cout << '|';
            }
        }
        std::cout << '\n';
    }
    
    void draw_board(const char board[])
    {
        draw_line(board, 0);
        std::cout << "-----\n";
        draw_line(board, 3);
        std::cout << "-----\n";
        draw_line(board, 6);
        std::cout << '\n';
    }
    
    /*
        displays prompt and returns true if player types y/Y
    */
    bool yes_no(const std::string& question)
    {
        std::cout << question << " [y/n] ";
        std::string response;
        while (std::getline(std::cin, response) && response.empty());
        std::cout << '\n';
        return tolower(response[0]) == 'y';
    }
    
    /*
        to convert between types
    */
    template <class T1, class T2>
    bool convert(const T1& from, T2& into)
    {
        std::stringstream ss;
        ss << from;
        return ss >> into;
    }
    
    /*
        gets a validated move from player
    */
    int get_player_move(const char board[])
    {
        int sel = 0;
        std::string response;
        std::cout << "Please enter your move no. ";
        while (!sel) {
            std::getline(std::cin, response);
            if (!convert(response, sel)) {
                std::cout << "Please enter a number!\n";
                sel = 0;
            }
            else if (sel < 1 || sel > 9) {
                std::cout << "Please enter a number in range 1-9\n";
                sel = 0;
            }
            else if (board[sel-1]) {
                std::cout << "This square is already taken! Try another.\n";
                sel = 0;
            }
        }
        std::cout << '\n';
        return sel-1;
    }
    
    void play_game()
    {
        char board[9] = { 0 };
        char players[2] = { 'X', 'O' };
        int player_to_move = 0;
        int human = yes_no("Do you want to let computer begin?") ? 1 : 0;
    
        if (player_to_move == human) {
            draw_board(board);
        }
    
        for (int move_count = 0; move_count != 9; ++move_count) {
            int move;
            if (player_to_move == human) {
                move = get_player_move(board);
            }
            else {
                move = pick_move(board, players[player_to_move], players[human]);
                std::cout << "The computer moved to " << move + 1 << "\n\n";
            }
            board[move] = players[player_to_move];
            draw_board(board);
    
            if (has_won(board, players[player_to_move])) {
                if (player_to_move == human) {
                    std::cout << "Well done! You win!\n\n";
                }
                else {
                    std::cout << "Sorry! The computer has beaten you...\n\n";
                }
                return;
            }
            player_to_move ^= 1;
        }
        std::cout << "Game over! It's a draw.\n\n";
    }
    
    int main()
    {
        srand(time(0));
        do {
            play_game();
        } while (yes_no("Play another game?"));
    }
    Last edited by anon; 05-24-2008 at 10:35 AM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  12. #12
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,304
    Here's a tidier way to check for all possible wins. Note that I'm assuming that instead of a 2D array theBoard[3][3] it's a 1D array theBoard[9]. This makes a lot of stuff easier.
    Code:
    bool Board::checkBoard(){
        const int wins[][3] = {
            {0,1,2},
            {3,4,5},
            {6,7,8},
            {0,3,6},
            {1,4,7},
            {2,5,8},
            {0,4,8},
            {2,4,6},
        };
        for (int i=0; i<sizeof(wins)/sizeof(wins[0]); ++i)
        {
            if (theBoard[wins[i][0]] == 'X' && theBoard[wins[i][1]] == 'X' && theBoard[wins[i][2]] == 'X'
            ||  theBoard[wins[i][0]] == 'O' && theBoard[wins[i][1]] == 'O' && theBoard[wins[i][2]] == 'O')
            {
                return won = true;            
            }
        }
        return false;
    }
    It's quite a simple, yet often overlooked solution, to use simple tables like this to represent things. You can then get away with just the one loop.
    Last edited by iMalc; 05-24-2008 at 09:11 PM.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  13. #13
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    i can't figure out why, but for some reason sometimes when you choose spot 6, spot 5 get overwritten.

  14. #14
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    What compiler/IDE do you use? Can you use the debugger?

    If not, can you post the code where it is supposed to write to spot 6 and the code where it is supposed to wrote to spot 5?

  15. #15
    Registered User Terran's Avatar
    Join Date
    May 2008
    Location
    Nashua, NH
    Posts
    100
    MINGW32/wxDevC++, and i can't ever get it to work.

    the whole code is posted above.

    From testing it seems that if space 5 has an '0' and space 7 has an 'X' and you choose space 6, then space 7 and 5 seem to switch?


    Edit: Debugging shows the erratic change, but i still can't be sure why.
    Last edited by Terran; 05-29-2008 at 04:14 PM.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 9
    Last Post: 07-15-2004, 03:30 PM
  2. Debugging-Looking in the wrong places
    By JaWiB in forum A Brief History of Cprogramming.com
    Replies: 1
    Last Post: 11-03-2003, 09:50 PM
  3. Confused: What is wrong with void??
    By Machewy in forum C++ Programming
    Replies: 19
    Last Post: 04-15-2003, 12:40 PM
  4. God
    By datainjector in forum A Brief History of Cprogramming.com
    Replies: 746
    Last Post: 12-22-2002, 11:01 AM
  5. Whats wrong?
    By Unregistered in forum C Programming
    Replies: 6
    Last Post: 07-14-2002, 01:04 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21