代码复审

一、概要

我们组选择的审核代码来自ShineTeam,他们要完成的项目是一个与众不同的游戏。经过代码复审，他们的代码可以完成预设的功能，代码可读性好，较易于维护，有相当数量的注释来进行说明，代码分成了单独的模块进行处理，有单元测试来保证模块的可用性。

二、设计规范部分

项目模式设计是，有少量数字的存在，不便于阅读和维护，代码如下：

void AimLaser::setLaserOpacity(int value)
{
    if(value < 0)
        value = 0;
    else if(value > 225)
        value = 225;
    m_laserModel->setOpacity(value);
}

没有说明对value设置225的上限的原因。

其余绝大多数地方均无数字，或者用变量来进行说明，部分代码如下：

const float GRADESPEED[] = { 300, 350, 400, 600, 900, 1500 };   //各等级光束对应的速度
const int   GRADEDAMAGE[] = {60, 100, 150, 250, 500, 1000};       //各等级光束对应的伤害
const char* GRADEFRAMENAME[] = { "Beam0000", "Beam0001", "Beam0002", "Beam0003", "Beam0004", "Beam0005" };

const float POINTINTERVAL = 2.0;
const float BEAMSPEEDRATES = 20;

由于他们选择了跨平台的设计，所以在移植方面没有问题。

在需要用到游戏引擎的部分，他们使用了cocos2d，并没有自己去写，在已经有可以调用的部分他们采用了调用而不是重新实现。

但在有的部分，例如重写一写方法时保留了原来的代码，将其注释掉而没有删除：

void AimLaser::reflectTrack()
{
    ////创建BatchNode,批量渲染laser，更快一点。
    //CCSpriteBatchNode* laserSegBatchNode = CCSpriteBatchNode::create("AimLaser.png");

    //每次都要恢复激光最大的范围值。
    float totalLaserLength= m_laserLength;

    Shield* hitShield      = NULL;
    Shield* pre_hitShield = NULL;

    CCPoint laserBegin      = m_tPosition; //碰撞时的激光的初始点
    CCPoint laserEnd      = laserBegin + totalLaserLength * ccpForAngle(CC_DEGREES_TO_RADIANS(-m_fRotation));//碰撞时激光的终点。
    float   leastLength      = totalLaserLength;  //记录碰撞点到上一个激光碰撞点的距离
    CCPoint hitPoint      = laserEnd;         //记录上一个碰撞点
    float   newRotation      = m_fRotation;       //初始点，光束的方向AimLaser相同
    
    while(totalLaserLength > 0)
    {
        CCArray* allShields = World::getCurrentWorld()->getShields();
        Shield*  shield;

        //检测碰撞，得到碰撞点最近的盾牌
        CCARRAY_TFOREACH(allShields, shield, Shield*)
        {
            Segment* shieldSeg        = (Segment*)shield->getBody();
            Segment* laserSeg        = new Segment(laserBegin, laserEnd);
            HitTestResult hitResult =  hitTest(shieldSeg, laserSeg);
            if(hitResult.isCross())
            {
                float hitLength = ccpDistance(hitResult.crossPoint, laserBegin);

                //找出离激光束最近的的碰撞点，并且该碰撞点所在盾牌不是上一次碰撞的盾牌（防止死循环）
                if(hitLength < leastLength && shield != pre_hitShield)
                {
                    leastLength = hitLength;
                    hitPoint    = hitResult.crossPoint;
                    hitShield    = shield;
                }
            }
        }

        //totalLaserLength保存的是激光被拦截之后的长度
        if(totalLaserLength < leastLength)
            totalLaserLength  = 0;
        else
            totalLaserLength -= leastLength;

        //计算出的被拦截的长度，用激光条束（laser）填满它
        while(leastLength > 0)
        {
            CCSprite* laser = CCSprite::createWithSpriteFrameName("AimLaser0000");
            float laserLength = laser->getContentSize().width;
            laser->setAnchorPoint(CCPointMake(0, 0.5));
            laser->setPosition(laserBegin);
            laser->setRotation(newRotation);
    
            int opacity = (int)(225 * (totalLaserLength + leastLength) / m_laserLength);
            laser->setOpacity(opacity);
            m_laserBatchNode->addChild(laser);

            //每填一个条束，应该填的长度减少相应的长度，直到剩余未填长度（leastLength）为0
            if(leastLength < laserLength)
            {
                laser->setTextureRect(CCRectMake(0,0, leastLength, laser->getContentSize().height));
                //laser->setContentSize(CCSizeMake());
                leastLength = 0;
            }
            else
                leastLength -= laserLength;
            //getParent()->addChild(laser);

            m_laserSeg->addObject(laser);
            laserBegin = laserBegin + laserLength * ccpForAngle(CC_DEGREES_TO_RADIANS(-newRotation));//碰撞时激光的终点。
        }

        //如果碰撞发生了，更新碰撞检测的状态,并且保存本次碰撞所在的盾牌，以防止下次遍历陷入死循环
        if(hitShield)
        {
            pre_hitShield    = hitShield;
            newRotation        = 2 * hitShield->getRotation() - newRotation; //newRotation表示的是上一个碰撞后得到的激光角度。这里更新它。
            laserBegin        = hitPoint; 
            laserEnd        = laserBegin + totalLaserLength * ccpForAngle(CC_DEGREES_TO_RADIANS(-newRotation));//碰撞时激光线段的终点。
            leastLength        = totalLaserLength;
        }
    }
}

//void AimLaser::reflectTrack()
//{
//    //每次都要恢复激光最大的范围值。
//    float totalLaserLength= m_laserLength;
//
//    Shield* hitShield      = NULL;
//    Shield* pre_hitShield = NULL;
//
//    CCPoint laserBegin      = m_tPosition; //碰撞时的激光的初始点
//    CCPoint laserEnd      = laserBegin + totalLaserLength * ccpForAngle(CC_DEGREES_TO_RADIANS(-m_fRotation));//碰撞时激光的终点。
//    float   leastLength      = totalLaserLength;  //记录碰撞点到上一个激光碰撞点的距离
//    CCPoint* hitPoint      = new CCPoint(laserEnd);         //记录上一个碰撞点
//    float   newRotation      = m_fRotation;       //初始点，光束的方向AimLaser相同
//    
//    while(totalLaserLength > 0)
//    {
//        CCArray* allShields = World::getCurrentWorld()->getShields();
//        Shield*  shield;
//
//        //检测碰撞，得到碰撞点最近的盾牌
//        CCARRAY_TFOREACH(allShields, shield, Shield*)
//        {
//            Segment* shieldSeg        = (Segment*)shield->getBody();
//            Segment* laserSeg        = new Segment(laserBegin, laserEnd);
//            HitTestResult hitResult =  hitTest(shieldSeg, laserSeg);
//            if(hitResult.isCross())
//            {
//                float hitLength = ccpDistance(hitResult.crossPoint, laserBegin);
//
//                //找出离激光束最近的的碰撞点，并且该碰撞点所在盾牌不是上一次碰撞的盾牌（防止死循环）
//                if(hitLength < leastLength && shield != pre_hitShield)
//                {
//                    leastLength = hitLength;
//                    hitPoint    = new CCPoint(hitResult.crossPoint);
//                    hitShield    = shield;
//                }
//            }
//        }
//
//        //计算出hitPoint.如果没有发生碰撞，hitPoint保持初始值laserEnd不变（即800 激光尾端）。
//        m_hitPointArray->addObject(hitPoint);
//
//        //totalLaserLength保存的是激光被拦截之后的长度
//        if(totalLaserLength < leastLength)
//            totalLaserLength  = 0;
//        else
//            totalLaserLength -= leastLength;
//
//        //如果碰撞发生了，更新碰撞检测的状态,并且保存本次碰撞所在的盾牌，以防止下次遍历陷入死循环
//        if(hitShield)
//        {
//            pre_hitShield    = hitShield;
//            newRotation        = 2 * hitShield->getRotation() - newRotation; //newRotation表示的是上一个碰撞后得到的激光角度。这里更新它。
//            laserBegin        = *hitPoint; 
//            laserEnd        = laserBegin + totalLaserLength * ccpForAngle(CC_DEGREES_TO_RADIANS(-newRotation));//碰撞时激光线段的终点。
//            leastLength        = totalLaserLength;
//        }
//    }
//}

多余代码可以清除掉。

三、代码规范部分

代码缩进符合要求，行宽合适，括号逻辑表达清楚，在仅有一行语句的情况依然用大括号进行分隔：

FarSpace::~FarSpace()
{ 
    World::getCurrentWorld()->unregEventHandler(this);

    CCArray* children = this->getChildren();
    CCObject* _obj;
    CCARRAY_FOREACH(children, _obj)
    {
        CC_SAFE_RELEASE(_obj);
    }  
}

也有一行语句却没有括号的情况：

        if(totalLaserLength < leastLength)
            totalLaserLength  = 0;
        else
            totalLaserLength -= leastLength;

命名十分规范，正确使用下划线，在需要的地方使用大写来命名。

对数据进行了封装，使用方法来对私有属性进行修改/读取。

int EnemyCraft::getMoney()
{
    return m_money;
}

void EnemyCraft::setMoney(int value)
{
    if(value >= 0)
        m_money = value;
}

有一定数量的注释，但有些程序段的注释有些缺乏，

void Beam::update(float dt)
{
    // update body area
    // 将上一个位置点记为起点，当前位置点记为终点
    CCAssert(getParent(), "Beam::update - not added to any layer");
    CCPoint newEnd = getBeamStripPosition();
    Segment* seg = (Segment*)m_body;
    seg->begin = seg->end;
    seg->end = newEnd;

    strikeTarget();
}

bool FarSpace::shipSpeedChanged(float scale)
{
    Dust* dust;
    CCArray* dustArray = Dust::getDustArray();
    CCARRAY_TFOREACH(dustArray, dust, Dust*)
    {
        dust->stopAllActions();

        float speedReal = dust->getSpeed() * scale;
        dust->setSpeed(speedReal);
        CCPoint position = dust->getPosition();

        CCFiniteTimeAction* moveTo = CCMoveTo::create((position.y + 50) / speedReal, ccp(position.x, -50));
        CCCallFunc* remove = CCCallFunc::create(dust,callfunc_selector(Dust::remove));
        dust->runAction(CCSequence::create(moveTo, remove, NULL));
    }
    return false;
}

有的地方仅一行注释或者完全没有，给阅读带来一定的困难。

四、具体代码部分

大多数地方没有错误处理，有的if并没有else来对其余情况进行处理，switch语句中的default语句采用了return 0或者break,也有未处理的情况。

void EnemyCraft::setBodyRadius(float value)
{
    if(value > 0)
    {
        m_bodyRadius = value;
        if(m_body && m_body->type == BTYPE_ROUND)
        {
            Round* round = (Round*)m_body;
            round->radius = m_bodyRadius;
        }
    }
}

Bullet* EnemyCraft::createBullet()
{
    if(!isDistanceSafe())
        return NULL;

    Bullet* bRet = NULL;
    switch(m_bulletType)
    {
    case BT_BEAM_1:
        bRet = Beam::create(1);
        break;
    case BT_BEAM_2:
        bRet = Beam::create(2);
        break;
    case BT_BEAM_3:
        bRet = Beam::create(3);
        break;
    case BT_BEAM_4:
        bRet = Beam::create(4);
        break;
    case BT_BEAM_5:
        bRet = Beam::create(5);
        break;
    case BT_BEAM_6:
        bRet = Beam::create(6);
        break;
    case BT_BOMB:
        bRet = Bomb::create();
        break;
    //case BT_CANON:
    //    bRet = Cannon::create();
 //       break;
    case BT_MISSILE:
        bRet = Missile::create();
        break;
    }
    if(bRet)
    {
        bRet->setSide(SIDE_ENEMY);
        bRet->addToWorld(getPosition(), 
            getAbsoluteShootDirection() + CCRANDOM_MINUS1_1() * m_shootScatter);
    }
    return bRet;
}

还有的地方使用了do{}while(0)的写法，但没有break。据解释，do{}while(0)是为了返回初始化失败的情况，而有的人在写的时候却没有加上break的部分。

bool Dust::init()
{
    bool bRet = false;

    do
    {
        ResourceManager* rm = ResourceManager::getSingleton();
        float scale  = 1.0f + CCRANDOM_0_1();

        // add visual - the mother point of the dust
        m_Dust = CCSprite::create(rm->getImageRes("Star"));
        m_Dust->setScale(scale);
        //m_Dust->retain();
        addChild(m_Dust);
         
        // add visual - tail of the dust
        m_Tail = createParticleSystem(rm->getParticle("DustTail"));
        m_Tail->setPositionType(kCCPositionTypeRelative);
        m_Tail->setSpeed(100);
        //m_Tail->retain();
        addChild(m_Tail);

        m_Speed = DUSTSPEED;
        scheduleUpdate();

        //World::getCurrentWorld()->regEventHandler(this);

        bRet = true;
    } while (0); 

    return bRet;
}

应该有

CC_BREAK_IF(m_Tail);
CC_BREAK_IF(m_Dust);

却没有。

他们使用了单元测试来保证模块的正确性，使用了断言来保证认为是真的模块一定正确。

以上截图为他们的单元测试程序，由于无完整工程的原因并没有运行。

五、效能

无完整工程文件，未进行效能的分析。

六、可读性

代码由不同的人来写的，有的部分注释比较多，有的部分注释比较少。从总体上来看，注释还是有点少，不过由于变量、函数的命名的规范性，可读性较强，具体可以参考前面添加

的代码。

七、可测试性

在写模块之前（之中），他们准备了对应的测试单元来进行测试以确保每部分的正确性。

ShineTeam 最终评分：90分。在工程的绝大多数部分都有着很好的代码风格，程序的结构也十分清楚，并且对大多数代码进行了测试。但在一些细节方面还有欠缺，注释的不足依然是一个问题。

posted @ 2012-12-12 21:21 76er 阅读(406) 评论(2) 收藏举报

刷新页面返回顶部

76er

代码复审

公告