可能会出现分段错误:我是否正确使用了“this->”运算符?

24

我在做一道家庭作业题目,有些问题需要帮助。如果你不想帮忙解决这个作业问题,我应该提醒一下,我的老师鼓励我们在完全无法解决问题时在这个网站上寻求帮助。另外,我已经独立完成了基本部分的任务,现在正在尝试一个可选的挑战问题。好了,进入正题!

由于我对OOP和C++都比较新,我无法理解“this->”操作符的含义。虽然我们课堂上没有讲过它,但我在其他地方见过它,并且我猜测它的用途。

对于这个任务,我必须创建一个基于控制台的井字棋游戏。只有挑战任务要求我们创建一个AI对手,而我们不会因此获得任何额外的学分,我只是想知道如何实现它。我正在学习如何使用minimax和游戏树等相关内容,但现在我只是想创建一个“随机选择空位”的函数。

我有一个名为TicTacToe的类,它基本上是整个程序。我将把与问题相关的部分贴在下面,但给我带来错误的那个子例程是:

void TicTacToe::makeAutoMove(){
    srand(time(NULL));
    int row = rand() % 3 + 1;
    int col = rand() % 3 + 1;
    if(this->isValidMove(row, col)){
        this->makeMove(row, col);
    }else{
        this->makeAutoMove();
    }
}

这个函数的唯一目的是在假设棋盘是空的情况下进行棋子移动。棋盘的设置如下:

char board[4][4];

当我打印它时,它看起来像:

   1  2  3
1  -  -  - 
2  -  -  -
3  -  -  -

问题在于,有时计算机会进行一些操作,导致我遇到了难以追踪的错误,因为这个函数具有随机性。我认为这是段错误,但我无法确定,因为在调试器中无法复制它。

我认为 "this->" 运算符作为指针,如果一个指针为空并且被访问,就可能出现这个问题。这是正确的吗?有没有方法可以解决这个问题?

我知道对于社区的许多成员来说,这可能是一个非常低级的问题,但只要不带嘲讽的话,我会非常感激您的帮助,不管这个问题有多么愚蠢。我正在学习,这意味着有时候我会问一些愚蠢的问题。

如果需要,这里是我的.cpp文件的更多内容:

TicTacToe::TicTacToe()
{
    for(int row = 0; row < kNumRows; row++){
        for(int col = 0; col < kNumCols; col++){
            if(col == 0 && row == 0){
                board[row][col] = ' ';
            }else if(col == 0){
                board[row][col] = static_cast<char>('0' + row);
            }else if(row == 0){
                board[row][col] = static_cast<char>('0' + col);
            }else{
                board[row][col] = '-';
            }
        }
    }
    currentPlayer = 'X';
}

char TicTacToe::getCurrentPlayer(){
    return currentPlayer;
}

char TicTacToe::getWinner(){
    //Check for diagonals (Only the middle square can do this)
    char middle = board[2][2];
    if(board[1][1] == middle && board[3][3] == middle && middle != '-'){
        return middle;
    }else if(middle == board[3][1] && middle == board[1][3] && middle != '-'){
        return middle;
    }

    //Check for horizontal wins
    for(int row = 1; row < kNumRows; row++){
        if(board[row][1] == board[row][2] && board[row][2] == board[row][3] && board[row][1] != '-'){
            return board[row][1];
        }
    }

    //Check for vertical wins
    for(int col = 1; col < kNumCols; col++){
        if(board[1][col] == board[2][col] && board[2][col] == board[3][col] && board[1][col] != '-'){
            return board[1][col];
        }
    }

    //Otherwise, in the case of a tie game, return a dash.
    return '-';
}

void TicTacToe::makeMove(int row, int col){
    board[row][col] = currentPlayer;
    if(currentPlayer == 'X'){
        currentPlayer = 'O';
    }else if(currentPlayer == 'O'){
        currentPlayer = 'X';
    }
}

//TODO: Make sure this works after you make the make-move function
bool TicTacToe::isDone(){
    bool fullBoard = true;
    //First check to see if the board is full
    for(int col = 1; col < kNumCols; col++){
        for(int row = 1; row < kNumRows; row++){
            if(board[row][col] == '-'){
                fullBoard = false;
            }
        }
    }

    //If the board is full, the game is done. Otherwise check for consecutives.
    if(fullBoard){
        return true;
    }else{
        //Check for diagonals (Only the middle square can do this)
        char middle = board[2][2];
        if(board[1][1] == middle && board[3][3] == middle && middle != '-'){
            return true;
        }else if(middle == board[3][1] && middle == board[1][3] && middle != '-'){
            return true;
        }

        //Check for horizontal wins
        for(int row = 1; row < kNumRows; row++){
            if(board[row][1] == board[row][2] && board[row][2] == board[row][3] && board[row][1] != '-'){
                return true;
            }
        }

        //Check for vertical wins
        for(int col = 1; col < kNumCols; col++){
            if(board[1][col] == board[2][col] && board[2][col] == board[3][col] && board[1][col] != '-'){
                return true;
            }
        }
    }
    //If all other tests fail, then the game is not done
    return false;
}

bool TicTacToe::isValidMove(int row, int col){
    if(board[row][col] == '-' && row <= 3 && col <= 3){
        return true;
    }else{
        //cout << "That is an invalid move" << endl;
        return false;
    }
}

void TicTacToe::print(){
    for(int row = 0; row < kNumRows; row++){
        for(int col = 0; col < kNumCols; col++){
            cout << setw(3) << board[row][col];
        }
        cout << endl;

    }
}

5
你因为a) 尝试回答问题和b) 提出一个合理的问题而得到了+1分 - 最近这样的情况不太常见。我会看看我能做些什么。 - Ed Heal
这个指针是内部指针;在正常情况下,唯一的方式使得这个指针为零是当创建的指针为NULL时。例如:如果 TicTacToe * ttt = NULL; - Gasim
2
应该以相反的顺序检查 board[row][col] == '-' && row <= 3 && col <= 3,否则可能会访问越界。 - dyp
1
@zsherman 没错,调用srand()会设置一些被rand()使用的隐藏全局变量,因此您应该只设置一次,并且程序的开头是一个很好的地方。如果您感兴趣,C++11提供了<random>库(用法示例),在几乎所有方面都比srand()/rand()更好。 - bames53
@zsherman - 您似乎既聪明又想学习。如果您愿意,可以直接联系我。我的电子邮件地址在这个网站上。只需点击我的用户名即可。 - Ed Heal
4个回答

10

一个普遍的前置声明:你几乎从不需要显式地使用this。在成员函数中,为了引用成员变量或成员方法,你只需命名该变量或方法,就像这样:

class Foo
{
  int mN;
public:
  int getIt()
  {
    return mN; // this->mN legal but not needed
  }
};
我认为 "this->" 操作符作为一个指针,如果一个指针是 NULL 并且被访问会导致此问题。这个说法正确吗?有没有办法修复它? this 是一个指针,确实如此。(实际上,它是一个关键字。)如果调用一个类的非静态成员函数,this 指向该对象。例如,如果我们调用上面的 getIt()
int main()
{
  Foo f;
  int a = f.getIt();
}

那么,this指向main()中的f

静态成员函数没有this指针。 this不能为NULL,并且您不能更改this的值。

C++中有几种情况可以使用this来解决问题,而其他情况则必须使用this。请参见帖子以查看这些情况的列表。


非常有用的建议,谢谢。不过我有一个好奇的问题,如果很少需要显式使用,那么什么情况下它实际上是必要的呢? - shermanzach
@zsherman:请参考这里(双关语)列出的原因清单,了解有时必须使用this的情况,以及其他情况下使用this是其中一种可能的解决方案。 - John Dibling
@EdHeal:另一个原因是类型相关的查找。请参见此处 - John Dibling
除了Ed Heal列出的内容之外,方法有时会返回this,例如:SomeClass* SomeClass::someMethod(...) { ... return this; }。这使我们能够将方法调用“链接”在一起:someObject->someMethod(...)->someMethod(...)->someMethod(...);。实际上,这就是<<和>> iostream运算符的实现方式,这就是它们可以链接在一起的原因(它们返回对this的引用)。 - David Young
感谢您提供详细的答案。虽然@dyp给出了修复我的代码的答案,但我将为回答我所问的具体问题提供正确的答案。希望其他有这个问题的人可以回顾并学习我所学到的! - shermanzach
显示剩余2条评论

9
我在未启用优化的情况下在 coliru 的 g++4.8.1 上重现了这个 bug。正如我在评论中所说,问题出在使用 srandtime 以及递归的结合上: time 的返回值通常是 Unix 时间(以秒为单位)。也就是说,如果您在同一秒钟内调用 time,您将获得相同的返回值。当使用此返回值来设定种子 srand(通过 srand(time(NULL)))时,您将在该秒钟内设置相同的种子。
void TicTacToe::makeAutoMove(){
    srand(time(NULL));
    int row = rand() % 3 + 1;
    int col = rand() % 3 + 1;
    if(this->isValidMove(row, col)){
        this->makeMove(row, col);
    }else{
        this->makeAutoMove();
    }
}

如果您不使用优化编译,或者编译器需要使用堆栈空间来执行每个makeAutoMove迭代,每次调用都会占用一点栈空间。因此,如果调用次数足够多,这将导致堆栈溢出(幸运的是,您来到了正确的网站)。
由于在同一秒钟内种子不会改变,所以对rand的调用也将在该秒钟内产生相同的值 - 对于每次迭代,第一个rand始终会产生某个值X,第二个则始终产生该秒内的某个值Y。
如果X和Y导致无效移动,则会出现无限递归,直到种子更改。如果您的计算机足够快,它可能会频繁调用makeAutoMove,从而占用足够的堆栈空间以在该秒钟内导致堆栈溢出。
请注意,不需要多次初始化rand使用的伪随机数生成器的种子。通常,您只需要初始化PRNG进行一次种子处理。然后,对rand的后续调用将产生伪随机数。
cppreference:randsrand

注意:堆栈溢出可能导致段错误。 - dyp
1
@zsherman 是的,没错。srand设定了一个全局状态(或线程本地)。注意,在C++11中有比srandrand更好的工具。可以在main函数中初始化PRNG,或者甚至在TicTacToe类中提供一些种子,使得类可以使用srand(例如在构造函数中),因此您可以通过传递相同的种子来重现AI移动序列。 - dyp

5

关于递归、效率、健壮编程以及如何保持谨慎的一点说明。

这是一个经过“清理”的有问题函数的版本。
请参考其他答案,了解原始版本出了什么问题。

void TicTacToe::makeAutoMove() {

    // pick a random position
    int row = rand() % 3;
    int col = rand() % 3;

    // if it corresponds to a valid move
    if (isValidMove(row, col)){
        // play it
        makeMove(row, col);
    }else{
        // try again
        makeAutoMove(); // <-- makeAutoMove is calling itself
    }
}

递归

通俗易懂地描述这段代码的作用:

  • 随机选择一个 (row, col) 对。
  • 如果该对代表一个有效的移动位置,则进行该移动。
  • 否则重试

调用 makeAutoMove() 确实是一种非常逻辑的 重试 方法,但从编程角度来看效率并不高。

每个新的调用都会在堆栈上分配一些内存:

  • 每个本地变量 4 字节(总共 8 字节)
  • 返回地址 4 字节

因此,堆栈消耗看起来像:

makeAutoMove             <-- 12 bytes
    makeAutoMove         <-- 24
        makeAutoMove     <-- 36
            makeAutoMove <-- 48
                         <-- etc. 

想象一下,假设您无意中在无法成功的情况下调用此函数(例如游戏已结束且没有有效移动可用)。
该函数将会无限地调用自身。在堆栈内存被耗尽并导致程序崩溃之前,只是时间问题。而且,鉴于普通PC的计算能力,崩溃将在眨眼之间发生。
这种极端情况说明了使用递归调用的(隐藏)成本。但即使函数最终成功执行,每次重试的成本仍然存在。
我们可以从中学到以下内容:
- 递归调用有成本; - 当未满足终止条件时,它们可能导致崩溃; - 很多递归调用(但不是全部)可以很容易地被循环替换,正如我们所看到的。
顺带一提,在dyp先生指出的注释中,现代编译器非常聪明,它们可以基于代码中的各种原因检测到某些模式,从而消除这种类型的递归调用。
尽管如此,您永远无法确定您特定的编译器是否足够智能,可以删除您的脚下的香蕉皮,因此如果问我的话,最好完全避免粗心。

避免递归

为了摆脱这种淘气的递归,我们可以这样实现再试一次

void TicTacToe::makeAutoMove() {
try_again:
    int row = rand() % 3;
    int col = rand() % 3;
    if (isValidMove(row, col)){
        makeMove(row, col);
    }else{
        goto try_again; // <-- try again by jumping to function start
    }
}

毕竟,我们不需要再次调用函数。跳回到函数起始点就足够了。这就是goto的作用。

好消息是,我们在不改变大部分代码的情况下摆脱了递归。
不那么好的消息是,我们使用了一个丑陋的结构来实现。

保留常规程序流程

我们不想保留那个笨重的goto,因为它会打破通常的控制流程,使代码非常难以理解、维护和调试*

然而,我们可以轻松地用条件循环替换它:

void TicTacToe::makeAutoMove() {

    // while a valid move has not been found
    bool move_found = false;
    while (! move_found) {

        // pick a random position
        int row = rand() % 3;
        int col = rand() % 3;

        // if it corresponds to a valid move
        if (isValidMove(row, col)){
            // play it
            makeMove(row, col);
            move_found = true; // <-- stop trying
        }
    }
}

好处:再见goto先生
坏处:你好move_found夫人

保持代码简洁

我们用一个标志位代替了goto语句。
现在程序流程不会被打断,但是代码变得更加复杂。

相对容易地,我们可以摆脱这个标志位:

    while (true) { // no way out ?!?

        // pick a random position
        int row = rand() % 3;
        int col = rand() % 3;

        // if it corresponds to a valid move
        if (isValidMove(row, col)){
            // play it
            makeMove(row, col);
            break; // <-- found the door!
        }
    }
}

好处:再见,move_found夫人
坏处:我们使用break,这只是一个被驯服的goto(类似于“跳到循环的结尾”)。

我们可以在此结束改进,但仍有一些让人烦恼的地方:循环的退出条件隐藏在代码中,这使得一开始很难理解。

使用显式的退出条件

退出条件对于确定代码是否有效非常重要(我们的函数陷入无限循环的原因正是有些情况下退出条件永远不满足)。

因此,尽可能清晰地突出退出条件总是一个好主意。

以下是使退出条件更明显的方法:

void TicTacToe::makeAutoMove() {

    // pick a random valid move
    int row, col;
    do {
        row = rand() % 3;
        col = rand() % 3;
    } while (!isValidMove (row, col)); // <-- if something goes wrong, it's here

    // play it
    makeMove(row, col);
}

您可以以稍微不同的方式完成它。只要我们实现了所有这些目标,就没有关系:
- 没有递归 - 没有多余的变量 - 有意义的退出条件 - 简洁的代码
当您将最新版本与原始版本进行比较时,您会发现它已经变异为完全不同的东西。
代码健壮性
正如我们所看到的,如果没有更多合法移动可用(即游戏已结束),则此函数永远无法成功。
这种设计可能有效,但它需要您的算法的其余部分确保在调用此函数之前正确检查了终局条件。
这使得您的函数依赖于外部条件,如果这些条件未满足,则会产生严重后果(程序挂起和/或崩溃)。
这使得此解决方案成为一个脆弱的设计选择。
偏执症来拯救
出于各种原因,您可能希望保留这种脆弱的设计。例如,您可能更喜欢与您的女友共进晚餐,而不是将晚上全部用于软件健壮性改进。
即使你的女友最终学会了如何应对极客,仍然会有一些情况下,你能想到的最好解决方案本身就存在潜在的不一致性,这是完全可以接受的,只要这些不一致性被发现并加以防范。
提高代码健壮性的第一步是确保危险的设计选择将被检测到,如果不能完全纠正,也需要进行部分修复。
一种做法是进入“偏执状态”,想象每个系统调用都会失败,任何函数的调用者都会尽其所能地使其崩溃,每个用户输入都来自一个狂热的俄罗斯黑客等。
在我们的情况下,我们不需要雇佣疯狂的俄罗斯黑客,也看不到任何系统调用。尽管如此,我们知道邪恶的程序员如何让我们陷入麻烦,因此我们将尽力防范。
void TicTacToe::makeAutoMove() {

    // pick a random valid move
    int row, col;

    int watchdog = 0; // <-- let's get paranoid

    do {
        row = rand() % 3;
        col = rand() % 3;

        assert (watchdog++ < 1000); // <-- inconsistency detection

    } while (!isValidMove (row, col));

    // play it
    makeMove(row, col);
}

assert是一个宏,如果作为参数传递的条件不满足,它将强制程序退出,并在控制台上显示一条消息和/或弹出窗口,显示类似于tictactoe.cpp line 238中的断言“watchdog++<1000”失败。
你可以将其视为一种方法,在检测到致命算法缺陷(即需要源代码全面修改的缺陷,因此保留这个不一致版本的程序没有多少意义)时退出程序。

通过添加看门狗,我们可以确保程序在检测到异常条件时显式退出,优雅地指示潜在问题的位置(在我们的情况下是tictactoe.cpp line 238)。

尽管重构代码以消除不一致可能很困难甚至不可能,但检测不一致通常非常容易且便宜。

条件不必非常精确,唯一的要点是确保您的代码在“合理”的一致上下文中执行。

在这个例子中,获取合法移动的实际尝试次数不容易估计(它基于累积概率来命中一个禁止移动的单元格),但是我们可以轻松地得出结论:在尝试1000次后仍未找到合法移动意味着算法出了严重的问题。
由于这段代码只是为了增加程序的健壮性,所以它不必高效。它只是一种从“为什么我的程序会挂起?!?”的情况转变为“该死,我肯定是在游戏结束后调用了makeAutoMove”(近乎)立即的认识的手段。
一旦你测试并证明了你的程序,如果你有非常好的理由(即,如果你的偏执检查导致严重的性能问题),你可以决定清理掉那些偏执代码,并在你的源代码中留下非常明确的注释,说明这个特定的代码应该如何使用。
实际上,有一些方法可以保持偏执代码的活力而不牺牲效率,但那是另外一个故事。
归根结底:
  • 习惯于注意代码中的潜在不一致性,特别是当这些不一致性可能会产生严重后果时
  • 尽可能确保您的代码中有尽可能多的部分可以检测到不一致性
  • 在您的代码中加入偏执检查以增加早期检测错误动作的机会

代码重构

在理想情况下,每个函数都应该给出一致的结果并使系统保持一致的状态。在现实生活中,除非您接受对创造力 进行一些限制,否则很少发生。

但是,如果您考虑了这些指南并设计了一个井字棋 2.0 ,那将是一个有趣的事情。 我相信您会在 StackOverflow 上找到很多有用的评论员。

如果您在所有这些抱怨中发现了一些感兴趣的点,请随时提问,并欢迎来到极客的精彩世界 :)
(kuroi dot neko at wanadoo dot fr)


* 在这个小例子中,goto看起来可能无害,但是你可以相信我:滥用goto会让你陷入痛苦的世界。除非你有非常非常好的理由,否则不要使用它。


哇!非常详尽的回复,提供了一些非常好的信息。仅仅浏览一下,我就学到了新东西。稍后会更加专注地查看。非常感谢。 - shermanzach
我有一个问题,@kuroi,你展示了看门狗的例子,你写了"assert (watchdog++ < 1000)",我不太明白这是什么意思? - shermanzach
1
assert是一个宏,如果作为参数传递的条件不成立,它会强制退出。这意味着“检查此条件并在不满足时退出程序”。换句话说,在程序检测到致命算法缺陷的情况下进行受控退出。当您以发布模式构建程序时,assert宏不会生成任何代码,因此允许在调试构建中运行偏执检查而不影响发布构建的性能。 - kuroi neko
我不喜欢中间步骤,但最终解决方案使用do-while循环相当优雅,加1分。但是,递归并不一定是这里的问题:通过优化,整数可以保留在寄存器中,并且递归可能会通过尾调用优化来解决。实际上,当使用优化编译我的测试案例时,尽管递归可能执行得更快,但似乎不再失败。 - dyp
@dyp 你说的递归是完全正确的。有些编译器足够聪明,可以从不规范程序员的脚下移除香蕉皮:) 然而,出于教育目的,我试图坚持基础知识。显示函数的中间版本的想法是为了介绍每个演变中的新(小)概念。这可能使一些中间版本看起来有些牵强。总体而言,我发现这个函数是一个相当好的教材基础。我会说,比你通常的阶乘/斐波那契教程更有趣。 - kuroi neko

5

以下是第一次修改的结果:

  1. 数组从零开始计数,因此在 rand() % 3 + 1; 这样的行中不需要 +1。
  2. this 表示当前对象的指针。通常你不需要使用它。例如:this->makeMove(row, col);makeMove(row, col); 的效果相同。
  3. char board[4][4]; 应该改为 char board[3][3];,因为你需要一个 3x3 的棋盘。参见上述第1点。
  4. board[row][col] = static_cast<char>('0' + row); - 你不需要强制转换,只需要使用 '0' + row 即可。
  5. 在代码的其余部分中,你需要考虑到第1点。
  6. 如果遇到段错误问题,请使用调试器,这是一项非常有用的技能。

无论如何,祝你学习愉快。很高兴在这个网站上看到一个热心于学习的新成员。


广告1)和3),OP似乎将行号+列号(要打印的)作为数组的一部分。虽然有点奇怪(静态输出是动态板数据的一部分),但似乎是正确的。 - dyp
@dyp - 我知道这样做有点正确,但养成这种习惯并不好,特别是当维度变得更大的时候。 - Ed Heal
是的,@dyp 是正确的,我确实将行号和列号作为数组的一部分,这就是为什么我使用了 rand() % 3 + 1; 而不是简单地使用 rand() % 3;虽然我不知道这是一个坏习惯,但我将来会避免这样做。谢谢! - shermanzach
2
@zsherman 好的编程关键在于学会以抽象、模块化、准确命名和(避免)依赖为思考方式。将行/列数字和棋盘内容放入同一数组中混合了抽象概念,使代码依赖于打印方式。您的“棋盘”不仅是游戏棋盘,还包括这些数字……最好将它们分开处理。编号应该是显示功能的一部分……它与游戏玩法无关。 - Jim Balter

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接