C ++ OOP Tic Tac Toe

Nov 11 2020

นี่คือการติดตามถึงคำถามของฉันที่นี่ มันไม่ได้เป็นการติดตามผล แต่เหมือนกับโครงการต่อไปของฉันหลังจากโครงการล่าสุด

ฉันสร้างเกม tic tac toe โดยใช้โปรแกรมเชิงวัตถุ

คุณทุกคนรู้อยู่แล้วว่านิ้วเท้า tic tac ทำงานอย่างไรดังนั้นฉันจะไม่เสียเวลาโดยอธิบายให้คุณทราบว่ามันทำงานอย่างไร

ฉันกำลังมองหาข้อเสนอแนะเกี่ยวกับทุกสิ่งที่สามารถทำให้ฉันเป็นโปรแกรมเมอร์ที่ดีขึ้นโดยเฉพาะโปรแกรมเมอร์ C ++ ที่ดีขึ้นรวมถึงวิธีการใช้คลาสฟังก์ชันที่ดีขึ้นและวิธีใช้ OOP อย่างถูกต้องและสิ่งเหล่านี้:

การเพิ่มประสิทธิภาพ
การปฏิบัติที่ไม่ดีและการปฏิบัติที่ดี
โครงสร้างรหัส
ฟังก์ชันและการตั้งชื่อตัวแปร
บัก
การปรับปรุงการใช้คลาสและฟังก์ชัน
วิธีการใช้ OOP อย่างถูกต้อง
อ้อยังจะเพิ่มความคิดเห็นอย่างไรให้ถูกต้อง
ฯลฯ

ขอบคุณมาก!

ฉันใช้ Visual Studio Community 2019 เวอร์ชั่น 16.7.7

Globals.h

#ifndef GUARD_GLOBALS_H
#define GUARD_GLOBALS_H

namespace
{
    enum class Players : char
    {
        PLAYER_X = 'X',
        PLAYER_O = 'O'
    };
}

#endif // !GUARD_GLOBALS_H

คณะกรรมการ h

#ifndef GUARD_BOARD_H
#define GUARD_BOARD_H

#include "player.h"

class Board
{
private:
    char board[9];
    // This is suppose to be a place to put the score
    // But I don't know how to implement it yet
    int scoreX{};
    int scoreO{};
public:
    Board();

    void printBoard() const;
    void markBoard(const size_t& choseNum, const char& player, bool& inputPass);
    char checkWin(bool& isDone, int& countTurn);
    void printWinner(bool& isDone, int& countTurn);
};

#endif // !GUARD_BOARD_H

board.cpp

#include "board.h"

#include <iostream>

// To set the board with numbers
Board::Board()
{
    int j{ 1 };
    for (int i = 0; i < 9; i++)
    {
        board[i] = '0' + j++;
    }
}

void Board::printBoard() const
{
    system("cls");

    std::cout << "   |   |  " << "\n";
    std::cout << " " << board[0] << " | " << board[1] << " | " << board[2] <<   "\tPlayer X: " << scoreX << "\n";
    std::cout << "___|___|__" <<                                                "\tPlayer O: " << scoreO << "\n";
    std::cout << "   |   |  " << "\n";
    std::cout << " " << board[3] << " | " << board[4] << " | " << board[5] << "\n";
    std::cout << "___|___|__" << "\n";
    std::cout << "   |   |  " << "\n";
    std::cout << " " << board[6] << " | " << board[7] << " | " << board[8] << "\n";
    std::cout << "   |   |  " << "\n\n";

}

// To change the board to which the player choose the number
void Board::markBoard(const size_t& choseNum, const char& player, bool& inputPass)
{
    char checkNum = board[choseNum - 1];
    // To check if the number that the player choose is available or not 
    if (checkNum != (char)Players::PLAYER_X && checkNum != (char)Players::PLAYER_O)
    {
        // To check if the number that the player input
        if (choseNum >= 1 && choseNum <= 9)
        {
            board[choseNum - 1] = player;
            inputPass = true;
        }
        else
        {
            std::cout << "CHOOSE THE AVAILABLE NUMBER!\nTRY AGAIN: ";
        }
    }
    else
    {
        std::cout << "SPACE HAS ALREADY BEEN OCCUPIED\nTry again: ";
    }
}

/*
There is probably a better way to do this. But, I don't know how tho
Maybe someday I could improve the checking for win but right now 
this is good enough

Also, there are a lot of magic number here such as 8, 2, 6 and 7.
I've tried to remove the magic number but I don't know how.
*/

// Check the board if there is player with parallel set or not
char Board::checkWin(bool &isDone, int &countTurn)
{
    /*
    I use middleboard and initialize it to board[4] because in order 
    for a player to win diagonally they have to acquire the 
    middle board first. So, I initialize middleboard to board[4]
    hoping it could remove the magic number

    and I initialize i to 0 and j to 8 because the checking is 
    begin from the top left corner-middle-bottom right corner 
    if it false then I add add 2 to i and substract 2 from j
    because now the checking is top right corner-middle-bottom left corner
    */

    // Check diagonal win
    size_t middleBoard = board[4];
    for (size_t i = 0, j = 8; i <= 2 && j >= 6; i+=2, j-=2)
    {
        // If all the board is occupied by the same player then the same player win
        if (middleBoard == board[i] && board[i] == board[j])
        {
            //This is suppose to add score, but I don't know how to implement it yet
            board[middleBoard] == (char)Players::PLAYER_X ? scoreX++ : scoreO++;
            isDone = true;
            return middleBoard; // To return the character of the player who won
        }
    }

    /*
    I initialize initialNum to 0 as a starting point for the checking. 
    Initilialized i to 1 and j to 2
    The checking is like this, top left corner-middle top-top right corner
    If it false then the I add 3 to initialNum to make middle left as the
    starting point, then add 3 to i and j so it the next checking is 
    middle left-middle-middle right, and so on
    */

    // Check horizontal win
    size_t initialNum = 0;
    for (size_t i = 1, j = 2; i <= 7 && j <= 8; i += 3, j += 3)
    {
        if (board[initialNum] == board[i] && board[i] == board[j])
        {
            board[initialNum] == (char)Players::PLAYER_X ? scoreX++ : scoreO++;
            isDone = true;
            return board[initialNum];
        }
        else
        {
            initialNum += 3;
        }
        
    }
    
    /*
    I reset the initialNum to 0 and initialized i to 3 and j 6 so 
    the first check will be like this: top left corner-middle left-bottom left corner
    if it fails then i add 1 to initialNum, i, and j, so the next check will be
    middle top-middle-middle bottom and so on
    */

    // Check vertical win
    initialNum = 0;
    for (size_t i = 3, j = 6; i <= 5 && j <= 8; i++, j++)
    {
        if (board[initialNum] == board[i] && board[i] == board[j])
        {
            board[initialNum] == (char)Players::PLAYER_X ? scoreX++ : scoreO++;
            isDone = true;
            return board[initialNum];
        }
        else
        {
            initialNum++;
        }
        
    }
    // If the countTurn is 8 then there're no place to occupy anymore, thus a draw
    if (countTurn == 8)
    {
        isDone = true;
        return 'D'; // As a check for printWinner() function
    }

    countTurn++;
}

// To print who's the winner or draw
void Board::printWinner(bool& isDone, int& countTurn)
{
    if (checkWin(isDone, countTurn) == 'D')
    {
        std::cout << "It's a Draw!\n";
    }
    else
    {
        std::cout << "Congratulations!\nPlayer " << checkWin(isDone, countTurn) << " won the game!\n";
    }
    
}

player.h

#ifndef GUARD_PLAYER_H
#define GUARD_PLAYER_H

#include "Globals.h"
#include "board.h"

class Board;

class Player
{
private:
    char mainPlayer;
    char secondPlayer;
    char turnPlayer = mainPlayer;

public:
    void choosePlayer(bool &choosePass);
    void movePlayer(Board& myBoard);
    void switchPlayer();
};

#endif // !GUARD_PLAYER_H

player.cpp

#include "player.h"
#include "board.h"

#include <iostream>
#include <random>

// To give a choice for the player if they want to be X or O
void Player::choosePlayer(bool& choosePass)
{
    char chosePlayer;
    std::cout << "Do you want to be player X or O? ";

    while (!choosePass)
    {
        std::cin >> chosePlayer;
        // If the player type X uppercase or lowercase then they will be
        // X and the computer will be O, vice versa
        if (chosePlayer == 'x' || chosePlayer == 'X')
        {
            mainPlayer = (char)Players::PLAYER_X;
            secondPlayer = (char)Players::PLAYER_O;
            choosePass = true;
        }
        else if (chosePlayer == 'o' || chosePlayer == 'O')
        {
            mainPlayer = (char)Players::PLAYER_O;
            secondPlayer = (char)Players::PLAYER_X;
            choosePass = true;
        }
        else
        {
            std::cout << "Invalid choice\n Try again: ";
        }
    }
}

// To make a player choose a number to which they want to occupy
void Player::movePlayer(Board &myBoard)
{
    size_t choseNum;
    bool inputPass = false;

    /*
    I make it turnPlayer != mainPlayer because if I make it
    turnPlayer == mainPlayer then the computer will make the first move
    I don't know why. Probably should find out the why. But it'll do for now
    */

    // If turnPlayer is not mainPlayer then it's the player's move
    if (turnPlayer != mainPlayer)
    {
        std::cout << "Player " << mainPlayer << " choose a number: ";

        while (!inputPass)
        {
            if (std::cin >> choseNum)
            {
                myBoard.markBoard(choseNum, mainPlayer, inputPass); //Go to markBoard function in board.cpp
            }
            else
            {
                std::cout << "Invalid input type (Type only number)\nTry again: ";
                std::cin.clear();                                                   // To clear the input so 
                std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // the player can input again
            }
        }
    }
    // If the turnPlayer is mainPlayer then it's the computer's move
    else
     {
         while (!inputPass)
         {
             // To make a random move for the computer
             std::random_device rd;
             std::mt19937 gen(rd());
             std::uniform_int_distribution<> distrib(1, 9);
             choseNum = distrib(gen);

             myBoard.markBoard(choseNum, secondPlayer, inputPass);
         }
     }
}

// To change turn, if the player finishes then the computer will make the move
void Player::switchPlayer()
{
    turnPlayer = (turnPlayer == mainPlayer) ? secondPlayer : mainPlayer;
}

main.cpp

#include "board.h"
#include "player.h"

int main()
{
    Board myBoard;
    Player mainPlayer;
    
    int countTurn{ 0 };
    bool choosePass = false;
    bool isDone = false;
    

    myBoard.printBoard(); // To print the initial board with numbered spaces
    
    while (!isDone)
    {
        if (!choosePass)
        {
            mainPlayer.choosePlayer(choosePass);
        }

        mainPlayer.movePlayer(myBoard);
        myBoard.printBoard();
        mainPlayer.switchPlayer();
        myBoard.checkWin(isDone, countTurn);
    }
    myBoard.printWinner(isDone, countTurn);
}

คำตอบ

6 AryanParekh Nov 12 2020 at 00:46

ควรมี`Globals.h`หรือไม่

ฉันไม่เห็นด้วย. Globals.hมีซิงเกิลenumที่มีความหมายต่อPlayerชั้นเรียนของคุณเท่านั้น เหตุใดจึงต้องสร้างส่วนหัวใหม่ ทำไมถึงเข้าไม่enum class Playersได้Player.cpp? นั่นเป็นไฟล์เดียวที่เข้าถึงเนื้อหาของไฟล์Players. ฉันเชื่อว่าสิ่งที่ดีที่สุดที่จะทำที่นี่คือการสร้างเนมสเปซที่ไม่ระบุตัวตนPlayer.cppและปล่อยให้มันคงอยู่ที่นั่น

// Player.cpp
namespace {
    enum class Players { ... };
}

นอกจากนี้โปรดใช้ความระมัดระวังขณะใช้เนมสเปซที่ไม่มีชื่อในไฟล์ส่วนหัว

ใช้std :: tolower

แทนที่จะเปรียบเทียบกับทั้งสองกรณีของอักขระให้ใช้std::tolowerเพื่อแปลงอักขระเป็นตัวพิมพ์เล็กโดยตรง สิ่งนี้จะแปลง

std::cin >> chosePlayer;

if (chosePlayer == 'x' || chosePlayer == 'X') {...}
else if (chosePlayer == 'o' || chosePlayer == 'O') {...}
else {...}

เป็น

std::cin >> chosePlayer;
chosePlayer = std::tolower(chosePlayer, std::locale());

if (chosePlayer == 'x' ) {...}
else if (chosePlayer == 'o') {...}
else {...}

^{#include <locale>}

โปรดทราบว่าในการป้อนอะไรก็ได้> 1 อักขระรหัสจะยอมรับตัวแรก ตัวอย่างเช่นหากผู้ใช้ป้อนcplusplus, การตั้งค่าในขณะนี้เพื่อchosePlayerc

ใช้สิ่งที่`enum class`คุณสร้างขึ้น

คุณได้สร้างการenumลบเวทมนตร์xและo. ทำไมคุณถึงยังใช้มันอยู่ที่นี่?

if (chosePlayer == 'x' ) 
else if (chosePlayer == 'o')

ใช้ค่าของenum class Playersที่นี่

ใช้`enum`ที่นี่

แม้ว่าบางคนอาจไม่เห็นด้วย แต่ฉันคิดว่าenumดีกว่าเมื่อเทียบกับenum classที่นี่ เหตุผลก็คือคุณไม่จำเป็นต้องร่ายค่าไปเรื่อย ๆcharเมื่อใดก็ตามที่คุณต้องการเปรียบเทียบenumและcharพิมพ์
หากจะปรากฏเฉพาะใน.cppไฟล์เดียวอย่างที่ฉันได้กล่าวไว้ก่อนหน้านี้คุณอาจไม่มีความขัดแย้งของชื่อ

enum Player : char { PLAYER_1 = 'x', PLAYER_2 = 'o' };

จาก `Player::chosePlayer()`

void Player::choosePlayer(bool& choosePass)
{
    char chosePlayer;
    std::cout << "Do you want to be player X or O? ";

    while (!choosePass)
    {
        std::cin >> chosePlayer;
        // If the player type X uppercase or lowercase then they will be
        // X and the computer will be O, vice versa
        if (chosePlayer == 'x' || chosePlayer == 'X')
        {
            mainPlayer = (char)Players::PLAYER_X;
            secondPlayer = (char)Players::PLAYER_O;
            choosePass = true;
        }
        else if (chosePlayer == 'o' || chosePlayer == 'O')
        {
            mainPlayer = (char)Players::PLAYER_O;
            secondPlayer = (char)Players::PLAYER_X;
            choosePass = true;
        }
        else
        {
            std::cout << "Invalid choice\n Try again: ";
        }
    }
}

หากคุณต้องการระบุว่าค่าที่ป้อนนั้นดีหรือไม่ดีเหตุใดคุณจึงส่งการอ้างอิงไปยังboolตัวแปร ทำไมไม่ส่งคืนtrueหากอินพุตดีและfalseหากอินพุตไม่เป็นเช่นนั้น การส่งผ่านการอ้างอิงเป็นการส่งผ่านตัวชี้โดยปริยายดังนั้นคุณกำลังส่งตัวชี้ไปยังตัวแปรบูลในฟังก์ชัน คุณจะต้องผ่านการอ้างอิงหากคุณใช้ตรรกะปัจจุบันของคุณ แต่สิ่งนั้นก็คือ

sizeof(bool) == 2
sizeof(bool*) == 8

ด้วยเหตุนั้นและเพื่อความเรียบง่ายฉันเชื่อว่าเพียงแค่กลับมาTrueหรือFalseจะดีกว่า

กำลังตรวจหาผู้ชนะ

อัลกอริทึมปัจจุบันของคุณในการตรวจสอบผู้ชนะนั้นยาวมากและอ่านยาก มีวิธีที่ดีกว่านี้ หัวข้อนี้จะช่วยให้จำนวนมากของข้อมูลที่เป็นประโยชน์เกี่ยวกับพวกเขา ง่ายที่สุด

constexpr int NB_WIN_DIR = 8;
constexpr int N = 3; // please think of a better name 

constexpr int wins[NB_WIN_DIR][N] {
    {0, 1, 2}, // first row
    {3, 4, 5}, // second row
    {6, 7, 8}, // third row
    {0, 3, 6}, // first col
    {1, 4, 7}, // second col
    {2, 5, 8}, // third col
    {2, 4, 6}, // diagonal
    {0, 4, 8}, // antidiagonal
};

for (int i = 0; i < NB_WIN_DIR ;i++)
{
    if (board[wins[0]] == board[wins[1]] and board[wins[1]] == board[wins[2]]) 
        return board[wins[0]];
}

คุณควรผ่านเมื่อ`const&`ใด

ฉันเห็นอาร์กิวเมนต์a const bool&and const size_t&function
เมื่อคุณควรส่งผ่านเป็นข้อมูลอ้างอิงคงที่

เมื่อคุณต้องการหลีกเลี่ยงสำเนาสำหรับวัตถุขนาดใหญ่

ดังที่ฉันได้กล่าวไว้ก่อนหน้านี้การส่งผ่านการอ้างอิงโดยปริยายจะส่งผ่านตัวชี้ แต่ปัญหาคือ

sizeof(bool) == 2
sizeof(bool*) == 8

sizeof(size_t) == 8 // depending on your machine, sometimes 4
sizeof(size_t*) == 8

ดังนั้นทางที่ดีที่สุดที่มันทำคุณความดีที่ไม่ทั้งหมดและอาจจะทำมากขึ้นที่ไม่ดี กฎง่ายๆของหัวแม่มือคุณไม่ต้องผ่านชนิดดั้งเดิมเช่นint, char, double, floatโดยแต่ไม่ผ่านโดยการอ้างอิงถ้าคุณมีสิ่งที่ต้องการconst&std::vector

อย่าเข้าใจฉันผิดคุณควรผ่านการอ้างอิงหากฟังก์ชันควรแก้ไขค่าดั้งเดิมของวัตถุ แต่ถ้านี่ไม่ใช่เจตนาให้ใช้สำหรับวัตถุขนาดใหญ่เท่านั้น

คิดโครงสร้างโค้ดของคุณใหม่

ฉันไม่ชอบชั้นนี้มาก

class Player
{
private:
    char mainPlayer;
    char secondPlayer;
    char turnPlayer = mainPlayer;

public:
    void choosePlayer(bool &choosePass);
    void movePlayer(Board& myBoard);
    void switchPlayer();
};

Playerชั้นเรียนของคุณไม่มีข้อมูลใด ๆ เกี่ยวกับผู้เล่นคนเดียว ฟังก์ชั่นสมาชิกทั้งหมดของคุณแก้ไขค่าของboardไฟล์. ทั้งหมดนี้เป็นของBoardชั้นเรียนของคุณจริงๆ ผู้เล่นที่เป็นจริงเพียงcharอย่างใดอย่างหนึ่งหรือo xแท้จริงแล้วไม่มีข้อมูลอื่นใดนอกเหนือจากนั้น สิ่งที่คุณควรทำก็แค่เป็นตัวแทนของผู้เล่นโดยใช้ enum เหมือนที่คุณทำอยู่แล้ว

enum Player { ... };

class Board{ 
      Player human;
      Player bot;  
};

botจะเป็นเครื่องคอมพิวเตอร์ที่จะเล่นกับคุณและhumanจะเป็นผู้ใช้จริง

สิ่งที่คิดว่าควรจะแสดงโดยใช้คลาสคือการเคลื่อนไหวง่ายๆ การเคลื่อนไหวมีสองสิ่ง

สี่เหลี่ยม
ผู้เล่น

ทุกที่ในโปรแกรมของคุณคุณได้ผ่านสองสิ่งนี้แยกจากกันทำไมไม่สร้างสิ่งง่ายๆstructที่จะเก็บมันไว้?

struct Move {
    int square;
    Player player;
}

ฉันได้เขียนตัวอย่างพื้นฐานเกี่ยวกับวิธีการจัดโครงสร้างเกมนี้ใหม่

class Game
{
    private:
        struct Move {
            Player player;
            int square;

            Move(const int square, const Player player)
                : square(square), player(player)
            {}
        };

        enum Player {
            PLAYER_1, PLAYER_2, NONE 
        };

        template < typename T, size_t N > using array = std::array < T, N  >;


        array < char, NB_SQ > board;
        Player human;
        Player bot;

        short int turns; // number of total moves played
    

    
        void computer_move();
        Move input_move() const;
        void make_move(const Move& move);
        bool validate_move(const Move& move);

        Player check_win() const;
        bool check_draw() const;

        void print_board() const;
        void new_game(); // choose whether the player plays 'x' or 'o' here
        
    public:
        void mainloop(){
            for (;;) {
                const Move& move = input_move();
                make_move(move);
                computer_move();

                if (check_win()) // ...
                if (check_draw()) // ...

            }
        }
        
        Game() { new_game(); }

};

int main() {
    Game game;
    game.mainloop();
}

เกี่ยวกับ `system("cls")`

โปรแกรมปัจจุบันของคุณจะไม่ทำงานบนระบบปฏิบัติการที่ไม่ใช่ Windows ในระบบอื่น ๆ clearส่วนใหญ่เป็นคำที่ เพื่อให้พกพาได้มากขึ้นคุณสามารถใช้คำสั่ง#ifdefเพื่อตรวจสอบระบบปฏิบัติการ

void clear_screen()
{
#ifdef _WIN32
    system("cls");
#else 
    system("clear");
#endif
}

อ่านเพิ่มเติม

3 pacmaninbw Nov 12 2020 at 05:47

ข้อสังเกตโดยรวม

โค้ดในmain()มีขนาดพอดีสวยและแน่นน่าอ่านมาก ข้อเสียอย่างเดียวmain()คือความคิดเห็นที่ไม่จำเป็นจริงๆ

ดูเหมือนว่าจะมีการพึ่งพาซึ่งกันและกันระหว่างบอร์ดและผู้เล่นในการออกแบบซอฟต์แวร์นี้เรียกว่าการมีเพศสัมพันธ์ที่แน่นหนาและโดยทั่วไปบ่งชี้ถึงการออกแบบที่ไม่ดี

ฉันเห็นเพียงหนึ่งอินสแตนซ์ของคลาสผู้เล่นและฉันคาดว่าจะเห็น 2 อินสแตนซ์สำหรับผู้เล่นแต่ละคน

ทำงานในการออกแบบวัตถุของคุณต่อไปเพื่อลบการมีเพศสัมพันธ์ที่แน่นและพยายามปฏิบัติตามหลักการเขียนโปรแกรมSOLID เรียนรู้รูปแบบการออกแบบเชิงวัตถุเช่นองค์ประกอบ

SOLID เป็นคำย่อที่ช่วยในการจำสำหรับหลักการออกแบบ 5 ประการที่มีจุดมุ่งหมายเพื่อให้การออกแบบซอฟต์แวร์มีความเข้าใจยืดหยุ่นและบำรุงรักษาได้มากขึ้น วิธีนี้จะช่วยให้คุณออกแบบวัตถุและคลาสได้ดีขึ้น

Single รับผิดชอบหลักการ - ชั้นควรจะมีความรับผิดชอบเพียงคนเดียวที่เป็นเพียงการเปลี่ยนแปลงที่จะเป็นส่วนหนึ่งของข้อกำหนดของซอฟต์แวร์ควรจะสามารถส่งผลกระทบต่อคุณสมบัติของชั้นเรียน
เปิดปิดหลักการ - รัฐหน่วยงานซอฟแวร์ (เรียนโมดูลฟังก์ชั่นอื่น ๆ ) ควรจะเปิดสำหรับการขยายปิด แต่สำหรับการปรับเปลี่ยน
Liskov ชดเชยหลักการ - วัตถุในโปรแกรมที่ควรจะเปลี่ยนได้กับกรณีของชนิดย่อยของพวกเขาโดยไม่ต้องเปลี่ยนความถูกต้องของโปรแกรมว่า
หลักการแยกการเชื่อมต่อ - ระบุว่าลูกค้าที่ไม่ควรถูกบังคับให้ขึ้นอยู่กับวิธีการก็ไม่ได้ใช้
พึ่งพาผกผันหลักการ - เป็นรูปแบบเฉพาะของ decoupling โมดูลซอฟต์แวร์ เมื่อปฏิบัติตามหลักการนี้ความสัมพันธ์การพึ่งพาแบบเดิมที่สร้างขึ้นจากโมดูลการตั้งค่านโยบายระดับสูงไปจนถึงโมดูลการพึ่งพาระดับต่ำจะถูกย้อนกลับดังนั้นการแสดงผลโมดูลระดับสูงโดยไม่ขึ้นกับรายละเอียดการใช้งานโมดูลระดับต่ำ

เปิดคำเตือนระดับสูงอย่าเพิกเฉยต่อคำเตือน

มีคำเตือน 2 คำเมื่อฉันรวบรวมและคำเตือนทั้งสองระบุปัญหาตรรกะที่อาจเกิดขึ้นในโค้ด

คำเตือนอย่างหนึ่งคือการสูญเสียข้อมูลในบรรทัดนี้:

            return middleBoard; // To return the character of the player who won

ในBoard::checkwin(). คำเตือนนี้เป็นเพราะโค้ดส่งคืนตัวแปรที่ประกาศsize_tเป็นไฟล์char.

คำเตือนที่สองก็เกี่ยวกับBoard::checkwin()คำเตือนnot all control paths return a valueซึ่งออกในบรรทัดสุดท้ายของฟังก์ชัน นี่อาจเป็นคำเตือนที่ร้ายแรงกว่า 2 ครั้งเนื่องจากมันบ่งบอกถึงปัญหาตรรกะที่เป็นไปได้ในโค้ด

ชอบแคสต์สไตล์ C ++ มากกว่าแคสต์สไตล์ C แบบเก่า

โค้ดบรรทัดต่อไปนี้ใช้แคสต์สไตล์ C แบบเก่า:

            board[initialNum] == (char)Players::PLAYER_X ? scoreX++ : scoreO++;

C ++ มีแคสต์ของตัวเองที่ให้คำเตือนและข้อผิดพลาดของคอมไพเลอร์ที่ดีกว่าซึ่ง ได้แก่static castsและdynamic casts. การร่ายแบบคงที่เกิดขึ้นในเวลาคอมไพล์และให้ข้อผิดพลาดหรือคำเตือนที่เป็นไปได้หากการแคสต์ไม่ปลอดภัย ในบรรทัดของโค้ดเหนือการร่ายแบบคงที่จะเหมาะสมกว่า

            board[initialNum] == (static_cast<char>(Players::PLAYER_X)) ? scoreX++ : scoreO++;

ชอบรหัสเอกสารด้วยตนเองมากกว่าความคิดเห็น

มีความคิดเห็นมากเกินไปในโค้ด สิ่งหนึ่งที่โปรแกรมเมอร์มือใหม่ไม่ทราบคือการดูแลรักษาโค้ดโค้ดที่คุณเขียนอาจใช้งานได้ 20 ปีขึ้นไปและมีความเป็นไปได้มากว่าคุณจะไม่ได้ทำงานกับ บริษัท นาน หากมีความคิดเห็นจำนวนมากในโค้ดความคิดเห็นจะต้องได้รับการดูแลรักษาเช่นเดียวกับตัวโค้ดเองซึ่งสามารถเพิ่มจำนวนงานที่ต้องทำได้เป็นสองเท่า ควรเขียนโค้ดการจัดทำเอกสารด้วยตนเองโดยใช้ชื่อตัวแปรคลาสและฟังก์ชันที่ชัดเจน ใช้ข้อคิดเห็นเพื่อการตัดสินใจในการออกแบบหรือนามธรรมระดับสูง หากฟังก์ชันต้องการสถานะโฟลว์พิเศษในบล็อกข้อคิดเห็นที่อยู่ข้างหน้าฟังก์ชัน

รหัสแห้ง

มีหลักการเขียนโปรแกรมที่เรียกว่าDon't Repeat Yourself Principleบางครั้งเรียกว่า DRY code หากคุณพบว่าตัวเองทำรหัสเดิมซ้ำหลายครั้งควรห่อหุ้มไว้ในฟังก์ชัน หากเป็นไปได้ให้วนซ้ำโค้ดที่สามารถลดการทำซ้ำได้เช่นกัน ฟังก์ชันBoard::checkWin()นี้มีโค้ดซ้ำซ้อนใน 3 ลูปที่ตรวจสอบการชนะ มีหลายวิธีในการแก้ไขปัญหานี้และมีการแนะนำวิธีที่ดีในคำตอบอื่น

ความซับซ้อน

ฟังก์ชันBoard::checkWin()ซับซ้อนเกินไป (ทำมากเกินไป) แทนที่จะส่งคืนอักขระBoard::checkWin()ควรส่งคืนค่าบูลีนที่ระบุว่าชนะหรือไม่ ฟังก์ชันอื่น ๆ ควรใช้การอัปเดตบอร์ดด้วยอักขระที่เหมาะสม not all control paths return a valueความซับซ้อนของฟังก์ชั่นนี้ได้นำไปสู่การแจ้งเตือน

เลขวิเศษ

มี Magic Numbers ในBoard::checkWin()ฟังก์ชันในแต่ละลูปที่ตรวจสอบว่ามีการชนะหรือไม่ควรสร้างค่าคงที่เชิงสัญลักษณ์เพื่อให้โค้ดอ่านง่ายขึ้นและดูแลรักษาได้ง่ายขึ้น ตัวเลขเหล่านี้อาจถูกใช้ในหลายสถานที่และสามารถเปลี่ยนได้โดยการแก้ไขเพียงบรรทัดเดียวทำให้การบำรุงรักษาง่ายขึ้น

ค่าคงที่ของตัวเลขในรหัสบางครั้งเรียกว่าMagic Numbersเนื่องจากไม่มีความหมายที่ชัดเจนสำหรับพวกเขา มีการอภิปรายเรื่องนี้อยู่ในStackOverflow

ขออภัย MyApp หยุดทำงาน ฉันจะแก้ปัญหานี้ได้อย่างไร?

ขอให้ผู้ใช้ป้อนข้อมูลจนกว่าพวกเขาจะตอบสนองที่ถูกต้อง

วิธีแก้ไข 'android.os.NetworkOnMainThreadException'

“ ความประหลาดใจน้อยที่สุด” และอาร์กิวเมนต์เริ่มต้นที่เปลี่ยนแปลงไม่ได้

คำสัญญาที่ชัดเจนในการก่อสร้างรูปแบบการก่อสร้างคืออะไรและฉันจะหลีกเลี่ยงได้อย่างไร

วิธีการหมุนกรอบข้อมูล?

กลยุทธ์การระบุตำแหน่งอย่างเป็นทางการสำหรับ Webdriver

การรวมแพนด้า 101

ฉันจะดึงข้อมูลจาก JSON ด้วย PHP ได้อย่างไร

“ ข้อผิดพลาดร้ายแรง: พบศูนย์โดยไม่คาดคิดขณะแกะค่าทางเลือก” หมายความว่าอย่างไร

ดีบักเกอร์คืออะไรและจะช่วยฉันวินิจฉัยปัญหาได้อย่างไร