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;
}

---

Quote:

---

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?

Quote:

---

Originally Posted by CornedBee

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!

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() % (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

Quote:

---

Originally Posted by Terran

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

Quote:

---

Originally Posted by matsp

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!

>> 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.

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.

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.

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.

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?"));
}

---

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.

i can't figure out why, but for some reason sometimes when you choose spot 6, spot 5 get overwritten.

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?

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.

You haven't changed the code at all since you posted it? Can you post the simplest inputs you give that will make the error happen every time?

of course i've changed it. The debugger narrowed it down to these two functions when the comp makes a move (as far as i can tell.)

I've changed int theBoard[3][3] to char theBoard[9]

Code:

---

        if(theBoard[4]=='X'){
            int ranCho;
            do {
                ranCho=((rand() % (9)) + 1);
            } while(theBoard[ranCho-1]=='X' && theBoard[ranCho-1]=='O' && ranCho != 0);
            return ranCho;
        }
        if(theBoard[4]=='O'){
            int ranCho;
            do {
                ranCho=((rand() % (9)) + 1);
            } while(theBoard[ranCho-1]=='X' && theBoard[ranCho-1]=='O' && ranCho != 0);
            return ranCho;
        }

---

These are at the bottom of the move logic to ad a little randomness if the game isn't about to be won. But it seems to be making illegal moves.

Quote:

---

theBoard[ranCho-1]=='X' && theBoard[ranCho-1]=='O' && ranCho != 0

---

The third part of this condition seems trivially true. Not that it would affect the outcome of the code.

The first and the second part, however, are mutually exclusive.

Also, ugly code duplication. Merge the conditions of the two ifs, the code in them is the same.

Well i did the code duplication to try and see if i could force the correct outcome, but i can't. So right now i'm piping the computers move choice through the function that checks to see if moves are legal. and that's working.

>> of course i've changed it.
I asked you to post the pertinent code, you said the whole code was posted above. But if the code has changed from what was posted above, then the above code isn't as relevant, right?

It's hard enough to read a bunch of code and figure out your error, but if the code is different than what is posted it makes it almost impossible.

Thank you for posting the updated code for spot 5 (I assume that's what that is), but what about the spot 6 code that fails?

I quote myself:

Quote:

---

The first and the second part, however, are mutually exclusive.

---

This means that the loop will NEVER repeat, and thus ALWAYS accept the first random value, whether it's valid or not.

Quote:

---

Originally Posted by CornedBee

I quote myself:

Quote:

---

The first and the second part, however, are mutually exclusive.

---

This means that the loop will NEVER repeat, and thus ALWAYS accept the first random value, whether it's valid or not.

---

But i'm, confused shouldn't it loop until ALL the conditions are met?

It loops while the statement is true. So it loops while theBoard[ranCho-1] is 'X' and it is 'O'. Since it cannot be both, that statement will never be true. Since that statement is never true, the loop will always only run once.

ahh gotcha, duh.