自动绘制棋盘图案的设计反馈

Question

我刚刚完成了斯坦福大学编程方法提供的第一次作业的第三项任务 (参见这里有足够的资源 )。

我只需要一些有用的反馈，说明我的代码如何更好(分解)，以及我的代码是否可以理解和可读性。

我也不确定前条件和后条件。谁能详细说明一下吗？当程序尝试该方法时，程序所处的前置条件和后条件是否只是实际方法的结果？

我需要所有我能得到的批评和反馈。

/* File: CheckboardKarel.java
 * ---------------------------
 * This program will basically go through an entire row and land checkers on every 2nd cell.
 * This process will be alternated on each row.
 */

import stanford.karel.*;

public class CheckerboardKarel extends SuperKarel {

    public void run() {
        while (facingWest() || facingEast()) {
            if (frontIsBlocked()) {
                turnKarelNorth();
            } //end if
            transverseRow();
            transverseEvenRow();
        } //end while
    }

    /*
     * Method: turnKarelNorth
     *  ---------------------- 
     * Pre-condition: The program is in a 8x1 (1 column) world and Karel has no choice but to turn north
     * Post-condition: This makes Karel turn left and face north
     */
    private void turnKarelNorth() {
        putBeeper();
        turnLeft();
        moveUpwards();
    }

    /*
     * Method: moveUpwards
     *  ---------------------- 
     *  This makes Karel lay down the beepers every 2 moves
     */


    private void moveUpwards() {
        while (facingNorth()) {
            move();
            move();
            putBeeper();
        }
    }
    /*
     * Method: transverseRow
     *  ---------------------- 
     *  Pre-Condition:Karel is facing east and is ready to place the beeprs on the first odd row
     *  Post-Condition: Karel places all the beepers every 2 steps on the first row
     */

    private void transverseRow() {
        while (facingEast()) {
            putBeeper();
            move();
            isFrontClear();
            isFrontBlocked();
        } // end while
    }

    /*
     * Method: isFrontClear (if statements)
     *  ---------------------- 
     *  This checks whether Karel is being obstructed.
     *  She takes anther move if the front is clear.
     *  Otherwise, if she is blocked, she executes placeBeeperOnNextRow
     */

    private void isFrontClear() {
        if (frontIsClear()) {
            move();
        } // end if
        else if (frontIsBlocked()) {
            placeBeeperOnNextRow();
        } // end else
    }

    /*
     * Method: isFrontBlocked (if statement)
     *  ---------------------- 
     *  This checks whether Karel is being obstructed.
     *  She puts the beeper and places a final beeper on the row.
     */

    private void isFrontBlocked() {
        if (frontIsBlocked()) {
            putBeeper();
            placeLastBeeperOnRow();
        }
    }

    /*
     * Method: placeLastBeeperOnRow (if statement)
     *  ---------------------- 
     *  This checks if Karel is facing east
     */

    private void placeLastBeeperOnRow() {
        if (facingEast()) {
            ascendOnOddColumn();
        }
    }
    /*
     * Method: placeBeeperOnNextRow (if)
     *  ---------------------- 
     *  This method makes Karel ascend at the end of an even column only if she is facing east
     */

    private void placeBeeperOnNextRow() {
        if (facingEast()) {
            ascendOnEvenColumn();
        }
    }

    /*
     * Method: ascendOnEvenColumn
     *  ---------------------- 
     *  This method makes Karel climb up the row and place a beeper on the new row
     */

    private void ascendOnEvenColumn() {
        turnLeft();
        move();
        putBeeper();
        turnLeft();
        move();
    }

    /*
     * Method: ascendOnOddColumn 
     * ---------------------- 
     * This method makes Karel
     * climb up the row and place a beeper on the new row
     */

    private void ascendOnOddColumn() {
        turnLeft();
        move();
        turnLeft();
    }

    /*
     * Method: transverseEvenRow
     *  ---------------------- 
     *  Pre-condition: Karel lays the checkers on the first row and is now on the 2nd row
     *  Post-condition: Karel lays the checkers alternately on every 2 steps
     */

    private void transverseEvenRow() {
        while (facingWest()) {
            move();
            putBeeper();
            isFrontClear();
            isFrontBlockedForEvenRow();
        }
    }
    /*
     * Method: isFrontBlockedForEvenRow
     *  ---------------------- 
     *  This checks whether Karel (on an even row) is being blocked
     *  It is in between the steps to help Karel move step-by-step without hitting a wall (which would break the loop)
     *   
     *  */

    private void isFrontBlockedForEvenRow() {
        if (frontIsBlocked()) {
            turnRight();
            move();
            turnRight();
        } //end if
    }

}

Answer 1

总的来说，您的代码看起来相当不错！我想说的是，如果您想要对您的能力进行更彻底的代码检查，那么您应该编写一些代码，而不是简单地调用这些外部方法。当所有方法只调用为您编写的其他方法时，很难很好地了解您是否知道良好的编程/Java习惯。

尽管如此，我的主要疑虑是与您的方法名称。通常，设计方法时都应该考虑到将来会有人使用它们。这意味着他们应该尽可能的自我解释和直觉。

例如：

private void turnKarelNorth() {
    putBeeper();
    turnLeft();
    moveUpwards();
}

作为一名开发人员，我认为turnKarelNorth()方法正是这样做的但它实际上并没有明确地做到这一点；你只知道，由于PDF中的具体问题，它总是会把她转向北方。它实际上是把她左转，根据她目前的状态，这可能是朝北的，也可能不是朝北的。

出于这个原因，我也会从这个方法中提取moveUpwards()调用。你不能保证，当它被称为，她将实际面对“向上”，这再次使你的设计有点不一致，更不灵活。而且，方法名turnKarelNorth()意味着方向调整，而不是位置调整。换句话说，开发人员会假设这只是改变了卡雷尔所面临的位置，而不是将她实际转移到另一个网格位置。

方法名称的一致性也是一个很大的问题。你有turnKarelNorth()，但是你有moveUpwards()。选择一个:要么是基本方向，要么是上下/左-右，但不要混合和匹配。它使您的代码更难遵循。

private void isFrontClear() {
    if (frontIsClear()) {
        move();
    } // end if
    else if (frontIsBlocked()) {
        placeBeeperOnNextRow();
    } // end else
}

要遵循Java约定，任何以"is“开头的方法都应该返回一个boolean。这是一个标准，它再次允许跨应用程序和整个语言的一致性。同样，这个名称意味着获取一段信息，而不是实际影响任何事情(正如您调用move()时所做的那样)。

我所说的一切都适用于你的大部分代码。总之，在设计方法时：

• 确保方法名称反映了方法的实际操作；
• 使您的方法尽可能灵活，使您的API更强大，并防止需要在未来完成重写；
• 有你的方法做一件事，把它做好；
• 在API中尽可能保持一致；以及
• 遵循Java命名约定

另外，作为附带说明，我认为您指的是遍历而不是横向。

Answer 2

有几种策略来生成路径，机器人可以采取这些策略来创建寻呼机模式。从机器人在两个示例中的最后位置判断，沿行的boustrophedon路径是正确的。这是个很好的开始。

由于这是在没有任何变量的情况下创建状态机的练习，所以在机器人的方向上编码状态信息的唯一好方法。(也有一些老生常谈的方法，但我们还是不要谈这个了。)因此，在每个函数调用之前和之后考虑机器人的状态是至关重要的。

您通常不首先检查一下这样的移动是否会从董事会的边缘掉下来就会move()。由于每次移动之前都应该进行检查，所以我将定义一个advance()函数：

/**
 * Advance along a boustrophedon path.
 *
 * Precondition: Karel is either
 *  (a) facing north, and the path is complete, or
 *  (b) facing east, and can advance east, or
 *  (c) facing east at the eastern edge, and needs to move one
 *      square north then turn to face west, or
 *  (d) facing west, and can advance west, or
 *  (e) facing west at the western edge, and needs to move one
 *      square north then turn to face east.
 *
 * Postcondition: Karel is either
 *  (A) facing north, and the path is complete, or
 *  (B) facing east, or
 *  (C) facing west.
 */
private void advance() {
    if (frontIsClear()) {
        move();
    } else if (facingEast()) {
        turnLeft();
        if (frontIsClear()) {
            move();
            turnLeft();
        }
    } else if (facingWest()) {
        turnRight();
        if (frontIsClear()) {
            move();
            turnRight();
        }
    }
}

一旦定义了该函数，run()函数就很简单，您不需要其他函数。由于这是一个家庭作业问题，我将省略解决办法。

增编

和看到该做什么一样重要的是认识到什么是不该做的。也许令人惊讶的是，定义一个用于遍历行的函数是毫无帮助的。为什么？想一想划出棋盘图案意味着什么--用六个字来描述任务。“董事会的边缘”并不是真正描述的一部分。

现在，考虑一下横穿一行意味着什么-它意味着你前进，直到你到达一个东西的边缘，然后做一个U-转到下一排。然后呢？你的下一个行动应该包括放下传呼机吗？答案是，这取决于！这就是为什么你的问题中有这么多代码:你必须处理向东和西行的奇数行和偶数行。基本的设计问题是行遍历功能以一个与任务无关的条件(击中板的边缘)结束(在另一个广场上设置一个传呼机)。