Thread: Checkers program feedback wanted - Jumping into C++ exercise

  1. #1
    Registered User
    Join Date
    Jul 2015
    Posts
    15

    Checkers program feedback wanted - Jumping into C++ exercise

    Hi,

    I'm learning C++ and am looking for some feedback on my checkers program. It works, but I just want to learn from your wisdom and find out if I have any bad coding habits to fix, tips, shortcuts etc...

    The question from the book:

    Chapter 10: Arrays

    Make a two-player checkers program that allow each player to make a move, and checks for legal moves and whether the game is over. Be sure to support kinging! Feel free to add support for any house rules that you use when you play. Consider allowing the user to choose the type of rules at program startup.
    My solution:

    Code:
    #include <iostream>
    #include <cstdlib>
    #include <vector>
    
    using namespace std;
    
    void move(vector<vector<char>>& board, int player);
    bool isLegal(vector<vector<char>>& board, int player, int x, int y, int j, int i);
    bool isKing(vector<vector<char>>& board, int player);
    bool isGameOver(vector<vector<char>>& board);
    vector<int> findJumpPos(vector<vector<char>>& board, int player);
    void jump(vector<vector<char>>& board, vector<int>& pos);
    void intialiseBoard(vector<vector<char>>& board);
    void displayBoard(vector<vector<char>>& board);
    
    int main()
    {
        // Create and deploy the game board
        vector<vector<char>> board(8, vector<char> (8));
        intialiseBoard(board);
    
        // Declare and initialise the number of players
        int player = 2;
    
        while(!isGameOver(board)) {
            cout << "CHECKERS" << endl;
            cout << "--------"<< endl;
            if(player == 2)
                player = 1;
            else
                player = 2;
    
            displayBoard(board);
            move(board, player); // Let the active player make their move
        }
    
        cout << endl << "Congratulations player " << player << "! You win." << endl;
    }
    
    // Allows the given player to make a move
    void move(vector<vector<char>>& board, int player)
    {
        bool isJump = false;
        vector<int> jumpPos = findJumpPos(board, player);
    
        // If there's a jump force current player to take it
        while(jumpPos.at(0) != 9) { // 9 indicates no jumps were found
            char jumpedPiece = board[jumpPos[2]][jumpPos[3]];
            isJump = true;
            jump(board, jumpPos);
            // Check for kings
            if((player == 1) && (isKing(board, player)))
                board[jumpPos[4]][jumpPos[5]] = 'K';
            else if((player == 2) && (isKing(board, player)))
                board[jumpPos[4]][jumpPos[5]] = 'Q';
            // Update the board
            system("CLS");
            cout << "CHECKERS" << endl;
            cout << "--------"<< endl;
            cout << endl << "Player " << player << ", you jumped a " << jumpedPiece;
            cout << " piece at " << static_cast<char> (jumpPos[3] + 65) << jumpPos[2] << endl << endl;
            displayBoard(board);
            cout << "Press enter to continue...";
            cin.ignore(256, '\n');
            cin.get();
            // Check if there is a double jump
            jumpPos = findJumpPos(board, player);
        }
    
        if(isJump)
            return;
    
        int invalidMove = 0;
        int x, y, j, i;
        char xTmp, jTmp;
    
        do {
            system("CLS");
            cout << "CHECKERS" << endl;
            cout << "--------"<< endl;
    
            if(++invalidMove > 1) {
                cout << endl << "Illegal move! Try again..." << endl << endl;
            }
    
            displayBoard(board);
    
            cout << endl << "Player " << player << " - " << endl;
            cout << "Choose X position of your piece -> ";
            cin >> xTmp;
            cout<< "Choose Y position of your piece -> ";
            cin >> y;
    
            x = static_cast<int> (xTmp - 65);
    
            cout << endl << "Choose X position of your chosen move -> ";
            cin >> jTmp;
            cout<< "Choose Y position of your chosen move -> ";
            cin >> i;
    
            j = static_cast<int> (jTmp - 65);
        } while(!isLegal(board, player, x, y, j, i));
        // Make the move
        board[i][j] = board[y][x];
        board[y][x] = 0;
        // Check for Kings
        if((player == 1) && (isKing(board, player)))
            board[i][j] = 'K';
        else if((player == 2) && (isKing(board, player)))
            board[i][j] = 'Q';
    }
    
    // Returns true when a player move's positioning is legal
    bool isLegal(vector<vector<char>>& board, int player, int x, int y, int j, int i)
    {
        bool legal = false;
    
        if((board[y][x] != 0) && (board[i][j] == 0)) {
            if((player == 1) && (board[y][x] == 'R') && ((j == (x + 1)) || j == ((x - 1))) && (i == (y + 1)))
                legal = true;
            else if((player == 2) && (board[y][x] == 'B') && ((j == (x + 1)) || j == ((x - 1))) && (i == (y - 1)))
                legal = true;
            else if((player == 1) && (board[y][x] == 'K') && ((j == (x + 1)) || j == ((x - 1))) && (i == (y + 1) || (i == (y - 1))))
                legal = true;
            else if((player == 2) && (board[y][x] == 'Q') && ((j == (x + 1)) || j == ((x - 1))) && (i == (y - 1) || (i == (y + 1))))
                legal = true;
        }
    
        return legal;
    }
    
    // Returns true when there is a new king
    bool isKing(vector<vector<char>>& board, int player)
    {
        bool king = false;
    
        for(int i = 0; i < board.size(); i++) {
            if((player == 1) && (board[7][i] == 'R'))
                king = true;
            else if((player == 2) && (board[0][i] == 'B'))
                king = true;
        }
    
        return king;
    }
    
    // Returns true when a player has won
    bool isGameOver(vector<vector<char>>& board)
    {
        bool over = false;
    
        int p1PiecesLeft = 0;
        int p2PiecesLeft = 0;
    
        for(int i = 0; i < board.size(); i++) {
            for(int j = 0; j < board.size(); j++) {
                if((board[i][j] == 'R') || (board[i][j] == 'K'))
                    p1PiecesLeft++;
                else if((board[i][j] == 'B') || (board[i][j] == 'Q'))
                    p2PiecesLeft++;
            }
        }
    
        if(p1PiecesLeft == 0)
            over = true;
        else if(p2PiecesLeft == 0)
            over = true;
    
        return over;
    }
    
    // Returns coordinates of the concerned grid positions when a player is obliged to jump
    vector<int> findJumpPos(vector<vector<char>>& board, int player)
    {
        vector<int> jumpPos(6, 9);
    
        for(int i = 0; i < board.size(); i++) {
            for(int j = 0; j < board.size(); j++) {
                if((player == 1) && (board[i][j] == 'R') && (i < 6) && (board[i + 1][j - 1] == 'B' || board[i + 1][j - 1] == 'Q') && (board[i + 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 1) && (board[i][j] == 'R') && (i < 6) && (board[i + 1][j + 1] == 'B' || board[i + 1][j + 1] == 'Q') && (board[i + 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j + 2);
                } else if((player == 2) && (board[i][j] == 'B') && (i > 1) && (board[i - 1][j - 1] == 'R' || board[i - 1][j - 1] == 'K') && (board[i - 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 2) && (board[i][j] == 'B') && (i > 1) && (board[i - 1][j + 1] == 'R' || board[i - 1][j + 1] == 'K') && (board[i - 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j + 2);
                } else if((player == 1) && (board[i][j] == 'K') && (i > 1) && (board[i - 1][j - 1] == 'B' || board[i - 1][j - 1] == 'Q') && (board[i - 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 1) && (board[i][j] == 'K') && (i > 1) && (board[i - 1][j + 1] == 'B' || board[i - 1][j + 1] == 'Q') && (board[i - 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j + 2);
                } else if((player == 1) && (board[i][j] == 'K') && (i < 6) && (board[i + 1][j - 1] == 'B' || board[i + 1][j - 1] == 'Q') && (board[i + 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 1) && (board[i][j] == 'K') && (i < 6) && (board[i + 1][j + 1] == 'B' || board[i + 1][j + 1] == 'Q') && (board[i + 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j + 2);
                } else if((player == 2) && (board[i][j] == 'Q') && (i < 6) && (board[i + 1][j - 1] == 'R' || board[i + 1][j - 1] == 'K') && (board[i + 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 2) && (board[i][j] == 'Q') && (i > 1) && (board[i - 1][j - 1] == 'R' || board[i - 1][j - 1] == 'K') && (board[i - 2][j - 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 2) && (board[i][j] == 'Q') && (i < 6) && (board[i + 1][j + 1] == 'R' || board[i + 1][j + 1] == 'K') && (board[i + 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j + 2);
                } else if((player == 2) && (board[i][j] == 'Q') && (i > 1) && (board[i - 1][j + 1] == 'R' || board[i - 1][j + 1] == 'K') && (board[i - 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i - 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i - 2);
                    jumpPos[5] = (j + 2);
                }
            }
        }
    
        return jumpPos;
    }
    
    // Forces a jump if there is one to be made
    void jump(vector<vector<char>>& board, vector<int>& pos)
    {
        if(board[pos[0]][pos[1]] == 'R') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'R';
        } else if(board[pos[0]][pos[1]] == 'B') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'B';
        } else if(board[pos[0]][pos[1]] == 'Q') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'Q';
        } else if(board[pos[0]][pos[1]] == 'K') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'K';
        }
    }
    
    // Initialises the game board
    void intialiseBoard(vector<vector<char>>& board)
    {
        // Player 1 side
        for(int i = 0; i < 3; i++) {
            for(int j = 0; j < board.size(); j++) {
                if((j % 2) == 0) {
                    if((i % 2) == 0) {
                        board[i][j] = 'R';
                    } else {
                        board[i][(j + 1)] = 'R';
                    }
                }
            }
        }
        // Player 2 side
        for(int i = 0; i < 3; i++) {
            for(int j = 0; j < board.size(); j++) {
                if((j % 2) == 0) {
                    if((i % 2) != 0) {
                        board[(i + 5)][j] = 'B';
                    } else {
                        board[(i + 5)][(j + 1)] = 'B';
                    }
                }
            }
        }
    }
    
    // Prints the board to the screen
    void displayBoard(vector<vector<char>>& board)
    {
        // X positions
        for(int k = 0; k < board.size(); k++)
            cout << "    " << static_cast<char> (k + 65);
    
        cout << endl << endl;
        // Y positions
        for(int i = 0; i < board.size(); i++) {
            cout << i << "  ";
            for(int j = 0; j < board.size(); j++) {
                if(board[i][j] != 0)
                    cout << "[ " << board[i][j] << " ]";
                else
                    cout << "[   ]";
            }
            cout << endl << endl;
        }
    }
    Thank you!

  2. #2
    Registered User
    Join Date
    May 2010
    Posts
    4,498
    The biggest thing I notice is that there appears to be a lot of code duplication that could easily be reduced. For example:
    Code:
    // Forces a jump if there is one to be made
    void jump(vector<vector<char>>& board, vector<int>& pos)
    {
        if(board[pos[0]][pos[1]] == 'R') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'R';
        } else if(board[pos[0]][pos[1]] == 'B') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'B';
        } else if(board[pos[0]][pos[1]] == 'Q') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'Q';
        } else if(board[pos[0]][pos[1]] == 'K') {
            board[pos[0]][pos[1]] = 0;
            board[pos[2]][pos[3]] = 0;
            board[pos[4]][pos[5]] = 'K';
        }
    }
    The first two lines in each section appear to be the same so it could possibly be reduced to:

    Code:
    // Forces a jump if there is one to be made
    void jump(vector<vector<char>>& board, vector<int>& pos)
    {
        board[pos[0]][pos[1]] = 0;
        board[pos[2]][pos[3]] = 0;
    
        if(board[pos[0]][pos[1]] == 'R') {
            board[pos[4]][pos[5]] = 'R';
        } else if(board[pos[0]][pos[1]] == 'B') {
            board[pos[0]][pos[1]] = 0;
        } else if(board[pos[0]][pos[1]] == 'Q') {
            board[pos[4]][pos[5]] = 'Q';
        } else if(board[pos[0]][pos[1]] == 'K') {
            board[pos[4]][pos[5]] = 'K';
        }
    }
    Which could possibly be possibly be reduced to:
    Code:
    void jump(vector<vector<char>>& board, vector<int>& pos)
    {
        board[pos[0]][pos[1]] = 0;
        board[pos[2]][pos[3]] = 0;
        board[pos[4]][pos[5]] = board[pos[0]][pos[1]];
    
        if(board[pos[0]][pos[1]] == 'B') {
            board[pos[0]][pos[1]] = 0;
    }

    There are several other places where you could probably reduce the duplication with a little thought.

    Jim

  3. #3
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    I feel like it'd be a lot easier for everyone if you instead wrote about your logic for the program itself. What were you going to design and how were you going to design it? Describe your algorithms and data structures.

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    7,364
    Another thing you could do would be to better explain in the code what your magic constants mean. For example, at first I thought jimblumberg's example would break the application because it makes changes even if board[pos[0]][pos[1]] isn't 'R', 'B', 'Q' or 'K'. But then I thought maybe those are the only possible values, so it's "ok". I tried to find what those constants meant in the code and didn't see it, so I don't know if that works or not.

  5. #5
    Registered User
    Join Date
    Jul 2015
    Posts
    15
    You were right initially Daved, jimblumberg's code would break my program. BUT, the point was very true and I was ultimately able to reduce that particular function to just:

    Code:
    // Sets the new position of the jumping piece and delete the jumped piece
    void jump(vector<vector<char>>& board, vector<int>& pos)
    {
        board[pos[4]][pos[5]] = board[pos[0]][pos[1]];
        board[pos[0]][pos[1]] = 0;
        board[pos[2]][pos[3]] = 0;
    }
    The main thing I will take away from this topic is that I need to comment better and perhaps properly document my program logic/algorithms if I intend to post it on here, in future. I've now moved well past this exercise in my book but just for your information on this one, R represents a player 1 piece (red), B represents a player 2 piece (black), K represents a player 1 king and Q represents a player 2 king (queen). Thanks all for the responses.
    Last edited by 1Sunny; 08-12-2015 at 03:32 AM.

  6. #6
    Registered User MutantJohn's Avatar
    Join Date
    Feb 2013
    Posts
    2,665
    The main thing I will take away from this topic is that I need to comment better and perhaps properly document my program logic/algorithms if I intend to post it on here, in future
    I don't know if it's just me or not but I go to a lot of different programming forums and I've gotten, well, lazier in my time.

    I typically don't read code unless it's very well-commented or is very short and plays well with its usage of whitespace. Otherwise, you get these really dense segments of code that make perfect sense to the creator but to anyone else, it seems like something they need to trudge through.

    Also, always make sure you use self-documenting variable names. No matter how long they are. Me being able to read your code and understanding it is 50x more important than the inconvenience of you having to type a few more characters.

    But I advocate that it's easier for people in forums if you just simply write about your approach with something like, "I was hoping to do X, Y and Z using algorithms A, B and data structures C and D."

  7. #7
    Registered User Alpo's Avatar
    Join Date
    Apr 2014
    Posts
    877
    Another place you can shrink your duplication is in the findJumpPos function. Since the beginning of every condition is checking a single value, you can use a switch statement to make it more readable and prevent checking the same conditions:

    Code:
    if((player == 1) && (board[i][j] == 'R') && (i < 6) && (board[i + 1][j - 1] == 'B' || board[i + 1][j - 1] == 'Q') && (board[i + 2][j - 2] == 0)) {                jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j - 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j - 2);
                } else if((player == 1) && (board[i][j] == 'R') && (i < 6) && (board[i + 1][j + 1] == 'B' || board[i + 1][j + 1] == 'Q') && (board[i + 2][j + 2] == 0)) {
                    jumpPos[0] = (i);
                    jumpPos[1] = (j);
                    jumpPos[2] = (i + 1);
                    jumpPos[3] = (j + 1);
                    jumpPos[4] = (i + 2);
                    jumpPos[5] = (j + 2);
                }

    Would become:

    Code:
    switch (board[i][j]) {
        case 'R':
            if ((board[i + 1][j - 1] == 'B' || board[i + 1][j - 1] == 'Q') && (board[i + 2][j - 2] == 0)) {
                // handle that
            } else if ((player == 1) && (board[i][j] == 'R') && (i < 6) && (board[i + 1][j + 1] == 'B' || board[i + 1][j + 1] == 'Q') && (board[i + 2][j + 2] == 0)) {
                // ... 
            }
    
    }
    Potentially you could also pass the board to a function which returns a named constant that this function could use to figure out what to do. Preferably I probably would have had the board contain enum values with names.

    I think it was Matticus who told me "The internal representation of data doesn't need to be the same as it's presentation to the user" when I was new. It's a good thing to keep in mind, it keeps your code more flexible.
    WndProc = (2[b] || !(2[b])) ? SufferNobly : TakeArms;

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 2
    Last Post: 08-28-2014, 09:06 PM
  2. Exercise from Jumping into C++ triggers trojan alert
    By droseman in forum C++ Programming
    Replies: 5
    Last Post: 04-25-2014, 05:23 AM
  3. Replies: 7
    Last Post: 10-03-2013, 03:27 PM
  4. Request for feedback -- exercise solution.
    By msh in forum C Programming
    Replies: 4
    Last Post: 07-07-2010, 12:48 PM
  5. Feedback wanted - DispHelper
    By anonytmouse in forum A Brief History of Cprogramming.com
    Replies: 3
    Last Post: 12-31-2003, 09:30 AM