Advertisement

Please HELP- simple game

Started by January 08, 2003 09:24 AM
4 comments, last by julienX 21 years, 10 months ago
I have recently started learning c++, my first language (for about a month now) and i just made a Tic-Tac-Toe program (in dos, i haven't moved to windows yet). I finished it and it works as I wanted it to, but when I look over my code, everything seems wrong. I feel i haven't structured my program very well. I don't want to carry this bad programming style and structure into windows (when i get to it). So could anyone please give me suggestions such as how to structure the program, ways to store data, and any other things. Oh yeah, have I used the Board class well(i bet not), is there any need for it? Here is the messy code:
      
//Tic-Tac-Toe game

//Created 1.8.03

#include <iostream.h>
#include <stdlib.h>

//Defines\Enums

#define PLAY  1
#define INTRO 2
#define EXIT  3
#define PLAYER1 100
#define PLAYER2 200
enum BOOL { FALSE,TRUE };
/////////////////////////


//Prototypes

int Menu();
void GameMain();
void Instructions();
/////////////////////////



//Class Declaration/////////////////////////

class Board
{
public:
void SetBoardStart();
void ShowBoard();
int CheckWin();  //Returns true if a player has won

void UpdateBoard(int place,int playerTurn);

private:
char board[3][3];
};
///////////////////////////////////////////


//Method declarations

void Board::SetBoardStart()
{
    //Sets all of board to blank space

    for (int i=0; i<3; i++)
    {
        for (int j=0; j<3; j++)
        {
        board[i][j] = ' ';
        }
    }
}

//Displays the updated board

void Board::ShowBoard()
{
system("cls");

cout <<"        -------------\n";
cout <<"        | " <<board[0][0] <<" | "<<board[0][1] <<" | ";
cout <<board[0][2] <<" |\n";

cout <<"        -------------\n";
cout <<"        | " <<board[1][0] <<" | "<<board[1][1] <<" | ";
cout <<board[1][2] <<" |\n";

cout <<"        -------------\n";
cout <<"        | " <<board[2][0] <<" | "<<board[2][1] <<" | ";
cout <<board[2][2] <<" |\n";
cout <<"        -------------\n";
}


void Board::UpdateBoard(int place,int playerTurn)
{
    char symbol;  //The symbol to place on the board

                  //Depends on the player turn

    if (playerTurn==PLAYER1)
    symbol = 'O';
    
    else 
    symbol = 'X';
    
        switch (place)
        {
        case 1: board[0][0] = symbol;
        break;
        case 2: board[0][1] = symbol;
        break;
        case 3: board[0][2] = symbol;
        break;
        
        case 4: board[1][0] = symbol;
        break;
        case 5: board[1][1] = symbol;
        break;
        case 6: board[1][2] = symbol;
        break;
        
        case 7: board[2][0] = symbol;
        break;
        case 8: board[2][1] = symbol;
        break;
        case 9: board[2][2] = symbol;
        break;
        
        default: cout <<"You lost your turn for nothing!\n";
        break;
        } //End switch       

} 
        

//Win Check

int Board::CheckWin()
{

if (board[0][0] == 'O' && board[0][1] == 'O' && board[0][2] == 'O'
||  board[1][0] == 'O' && board[1][1] == 'O' && board[1][2] == 'O'
||  board[2][0] == 'O' && board[2][1] == 'O' && board[2][2] == 'O'

||  board[0][0] == 'O' && board[1][0] == 'O' && board[2][0] == 'O'
||  board[0][1] == 'O' && board[1][1] == 'O' && board[2][1] == 'O'
||  board[0][2] == 'O' && board[1][2] == 'O' && board[2][2] == 'O'

||  board[0][0] == 'O' && board[1][1] == 'O' && board[2][2] == 'O'
||  board[0][2] == 'O' && board[1][1] == 'O' && board[2][0] == 'O')
{
    return 1; //Returns 1- player 1 has won

}

if (board[0][0] == 'X' && board[0][1] == 'X' && board[0][2] == 'X'
||  board[1][0] == 'X' && board[1][1] == 'X' && board[1][2] == 'X'
||  board[2][0] == 'X' && board[2][1] == 'X' && board[2][2] == 'X'

||  board[0][0] == 'X' && board[1][0] == 'X' && board[2][0] == 'X'
||  board[0][1] == 'X' && board[1][1] == 'X' && board[2][1] == 'X'
||  board[0][2] == 'X' && board[1][2] == 'X' && board[2][2] == 'X'

||  board[0][0] == 'X' && board[1][1] == 'X' && board[2][2] == 'X'
||  board[0][2] == 'X' && board[1][1] == 'X' && board[2][0] == 'X')
{
    return 2;  //Returns 2- player 2 has won

}

return 0;
}


///////////////////////////////////////////////////////////////////////

//Main- program starts here

int main()
{

int exit=FALSE,choice;

    while (!exit)
    {
    choice=Menu();
        
        switch (choice)
        {
        case INTRO: Instructions();
        break;
        
        case EXIT: exit=TRUE;
        break;
        
        case PLAY: GameMain();
        break;
        
        default: break;
        } //End switch

    
    } //End while


system("cls");
cout <<"Thanks for playing!\n";
system("PAUSE");

return 0;
} //End main


///////////////////////////////////////////////////////////////////////


//Non member function declarations

int Menu()
{
    system("cls");
    int menuChoice;
    cout <<"*** Main Menu ***\n";
    cout <<"(1) Play Tic-Tac-Toe\n";
    cout <<"(2) Instructions\n";
    cout <<"(3) Exit\n";
    cin >>menuChoice;
    
    return menuChoice;
}

void Instructions()
{
    system("cls");
    cout <<"This is a very simple Tic-Tac-Toe game.\n";
    cout <<"It is meant to be played with two people, taking turns each round\n\n";
    cout <<"To place your symbol on the board, just type the number of the box to place it in.\n";
    cout <<"For example: type 1 to place you symbol in the top-left corner of the board.\n";
    cout <<"\nHere is are the numbers of each place on the board:\n\n";
    cout <<"I am trying to figure out better ways to handle data.\n\n";
    system("PAUSE");
}


//Main game loop

void GameMain()
{
    Board theBoard;
    theBoard.SetBoardStart(); //Sets the board to blank

    
    int turnCounter=0; //If even- player 1 turn

                       //If odd- player 2 turn

    int playerTurn;
    char *playerName;
    int place; //Sets O or X on the game board

    
    while (TRUE)
    {
        //Sets player turn: even or odd?

        if (turnCounter %2 == 0)
        {
        playerName = "Player 1";
        playerTurn = PLAYER1;
        }
    
        else 
        {
        playerName = "Player 2";
        playerTurn = PLAYER2;
        }

        theBoard.ShowBoard(); //Shows the board

        
        int winCheck = theBoard.CheckWin(); //Win Check on board

        
        if (winCheck==1)
        {
                cout <<"Player 1 has won!\n";
                system("PAUSE");
                break; //exit game loop

        }
        
        if (winCheck==2)
        {
                cout <<"Player 2 has won!\n";
                system("PAUSE");
                break; //exit game loop

        }
        
        cout <<"\n\n" <<playerName <<": ";
        cin >>place; //Accepts input from user

        
        theBoard.UpdateBoard(place,playerTurn);
        
        turnCounter++;
    }
    
} //End GameMain

      
I'm sorry if I'm asking much.. [edited by - julienX on January 8, 2003 10:58:42 AM] [edited by - julienX on January 8, 2003 11:01:06 AM]
chacha
Please feel free to give your comments and suggestions on any part of my horrible code, and thankyou for your time.

Any feedback would help me out a lot
chacha
Advertisement
Well for starters I would move the class declaration in a separate header file.

Then maybe create a new class called Checkers(and put the declaration in a seperate header file). That class has Board as a private member. All the game logic would go into Checkers for example, menu, instructions...

Maybe you should work on your coding style(considering mine :-) ). You have a new line here and there, indent is unreadable at best, but your comments are good. I am sure your coding style will evolve as you go along though.

Knowing that you have only 1 month experence in C++ I find this code acceptable if not good, given your experience.

Hope this helps...
Friðrik Ásmundsson
-> your switch statements, for values such as EXIT, PLAY... Should take values from an enum, instead of defining everything using the preprocessor.

-> move all declarations to a header file (or more)

-> indent correctly everything, using one and only one coding style, be it :

statement() {    Code;}(K&R style / java style)statement(){    Code;}orstatement()  {    Code;  } 


->Store as many variables as possible in your class, as member variables. For instance, current player turn... that way, if another funciton needs to access it, you won''t have to turn it global...

->Implement your c-style functions into a wrapper class with Menu(), etc... as well as a Board member variable (that way you don''t need to create it). All you''ll have to do is create an instance of that wrapper in the main() function and call whatever function required (maybe even include a GetInput() funtion replacing the Menu() one in the Board class, or something).

->Declare ONE OBJECT (or group of objects) per file... (say, Board class in Board.h, Global functions in Global.h), define one object per file (all global functions in Global.cpp, all Board member functions and related functions in Board.cpp)

->Comment your code by explaining (two lines) what EACH function returns/accepts as argument/modifies/does(non-explicit functions only). For instance, how would I know CheckWin returns the NUMBER of the winning player?

->[case-specific] which player is using ''X''? ''O''? maybe including an enum{PLAYER_O, PLAYER_X} (more explicit than 1, 2) would have been more useful than that BOOL {true, false] one...

Otherwise the code looks clean to me. i wish I had coded my first games like that... or my TicTacToe game, also...

ToohrVyk
-------------
Extatica - a free 3d game engine
Available soon!
Click here to learn more
Getting it to work as you want after a month is definitely an accomplishment.
Read other peoples code and you''ll get there.
Don''t worry too much about style yet.
one thing to consider is the use of negative logic, i.e., your use of "exit" as a name for your loop termination variable causes you to code the while loop as while(!exit). i know it seems like a small point, but once you get into more complicated tests, using negative logic can make your code unreadable. so, it''s best to get in the habit of always choosing variable names that will allow your code to be more readable. and positive tests are always more readable than negative tests. maybe switching to a variable name of "running" would make more sense in this case. so your looping logic would then be:

  while( running ){   ...      case EXIT: running = false; break;   ...}  

also, bool is a valid variable type in C++, so you can use it in place of your int/TRUE/FALSE variables.

This topic is closed to new replies.

Advertisement