对《敏捷软件开发:原则、模式与实践》中保龄球程序重构的一些思考

  前几天看了《敏捷软件开发:原则、模式与实践》中第六章:一次编程实战,文章中主要描述了一对开发人员进行一次记录保龄球比赛成绩程序的开发过程.仔细研究之后,发现一个问题,拿出来和大家讨论讨论.(有兴趣的同学可以先看一看书中第六章的内容)

  简单来说,该程序是用来计算保龄球比赛分数的,程序计算分数的功能不多说了.这里主要关注程序判断"当前在第几轮"(currentFrame)这一变量上.

  先来看单元测试吧: (对保龄球规则不熟的同学请点击这里)

    @Test
    public void testFourThrowsNoMark(){
        game.add(5);
        game.add(4);
        game.add(7);
        game.add(2);
        assertEquals(18, game.score());
        assertEquals(9, game.scoreForFrame(1));
        assertEquals(18,game.scoreForFrame(2));
    }

  这个测试表达的意思是:当分数增加4次后(也就是说投掷4次球后),判断当前的比赛总分数,第一轮分数,第二轮分数.

  Game类中的add方法:(我们不用关注itsScorer.addThrow方法)

    public void add(int pins) {
        itsScorer.addThrow(pins);
        adjustCurrentFrame(pins);
    }

  add方法中,adjustCurrentFrame方法主要是根据当前分数(pins)来调整currentFrame这一变量

    private void adjustCurrentFrame(int pins) {
        if (firstThrow) {
            if (!adjustFrameForStrike(pins)){
                firstThrow = false;  // boolean, indicate first throw or not
            }
        } else {
            firstThrow = true; // notice, firstThrow turn to 'true'
            addOneFrame();
        }
    }

    private boolean adjustFrameForStrike(int pins) {
        if (pins == 10) {
            addOneFrame();
            return true;
        }
        return false;
    }
    private void addOneFrame(){
        currentFrame = Math.min(10,currentFrame+1) 
    }

  在上面的代码中,firstThrow表示"是否是一轮中的第一次投掷",如果是第一次投掷,则判断是否10分,如果是10分,则调用addOneFrame使currentFrame增1;如果不是10分,则firstThrow设为false.当再次调用adjustCurrentFrame时,由于firstThrow是false,则首先将firstThrow设为true,然后currenetFrame增1.(通过min操作保证currentFrame不会超过10)

  通过测试后,作者开始重构,最终产生以下代码:  

    private void adjustCurrentFrame(int pins) {
        if (lastBallInFrame(pins))
            advanceFrame();
        else
            firstThrowInFrame = false;
    }

    private boolean lastBallInFrame(int pins) {
        return strike(pins) || !firstThrowInFrame;
    }

    private boolean strike(int pins) {
        return (firstThrowInFrame && pins == 10);
    }
    private void advanceFrame(){
        currentFrame = Math.min(10,currentFrame+1)
    }

  不得不说,上面的代码可读性更好了.但仔细阅读之后,发现一个问题,firstThrowInFrame(就是第一个版本的firstThrow)只能从true变为false,而无法从false变为true.当firstThrowInFrame为false时,产生的一个结果就是:add方法调用adjustCurrentFrame方法时,lastBallInFrame永远返回true,而strike永远为false.也就是说,当第一次投掷不是10分时,程序会认为你已经结束了本轮投掷,currentFrame增1(实际上你还没有结束本轮);本轮第二次投掷,程序会认为这是新一轮的投掷,并且再次增加currentFrame,导致currentFrame计算错误.

  可是我把代码敲了一遍,运行了测试,全绿通过...这是为什么呢?

  我想应该有三个原因:

  1.因为currentFrame不影响计算当前总分数和每局分数: 作者使用数组保存每次投掷分数,在计算分数时使用currentFrame依次累加数组.因为数组中的全部元素一开始被初始化为0,  虽然currentFrame错误,但是正确分数加上后面多加的错误分数也不会影响结果,所以全绿通过.(比如分数是1,2,3,4,5,6,0,0,0,0.假设根据currentFrame需要把数组中的内容全部叠加,但因为包含了前6个正确的分数,所以后面的0加上也不会影响结果)

  2.advanceFrame方法中保证currentFrame最大不超过10:这是保证计算总分数时,迭代保存分数数组不会越界.

  3.测试中不包含对currentFrame的测试:因为作者认为Game对象不需要向外界提供该方法,所以也就没有相对应的测试.

  说的这里,其实现在的currentFrame已经不重要了,因为按照测试来看,当前程序完全正确.但是通过代码来看,至少我认为程序还是有瑕疵的,毕竟程序错误的计算了currentFrame.

  于是,我又一次遇见了同样的问题:代码可以通过单元测试进行验证,那么单元测试应该如何验证?或者说我们如何保证写出正确的单元测试呢?

  虽然和不少人讨论过这个问题,但我还是没有十足的把握把它写出来,我想应该是时机不到吧(以后一定要写一篇文章和大家讨论一下)

  总之,我写这篇文章的目的就是告诉大家:就算测试全绿了,系统中也许还存在着一些我们看不见的瑕疵,重构时,一定要小心小心再小心!!!

-----------------------------------------------------------------------------------------------------------------------------

  ps:我本想给Uncle Bob大叔发信问问,结果在网上搜了半天也没有找到Bob大叔的邮箱(有知道的同学给我留言).搜了很久,总算在一个论坛找到了一个靠谱的Bob,给他发了邮件,静候回信吧。

原文地址:https://www.cnblogs.com/iammatthew/p/2053051.html