狗屎代码鉴赏

发表于 更新于 分类于 阅读次数：
整理若干可读性差、多余封装、糟糕命名、重复逻辑与魔法数字等反面代码示例及点评。
1
2
3
4
5
6
7
8
9
10
11
12
13
case KeyEvent::KEYCODE_STOP:
{
    NGInt ret = showModalView (LAUNCHER_CLT_STOPSHORTCUT, 0, 0);
    if (ret == 1)
    {
        raw_uart_log("%s Shutdown key called,line:%d\r\n",__func__,__LINE__);
        EntryShutDown (NULL);
    }
    else
    {
        m_baseView->show ();
    }
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
// evb/src/gui/mgapollo/apps/phonebook/include/phonebookitem.h

class PhoneBookItem : public ItemView {
    protected :
        ImageView*                      m_markIcon; // 多选框选中
		ImageView*                      m_markIcon_uncheck; // 多选框未选中
}

void PhoneBookItem::setMarked(NGBool bmarked)
{
    if(bmarked){
        m_markIcon->show();
	    m_markIcon_uncheck->hide();
    }
    else{
        m_markIcon->hide();
		if(m_bMarkState)
			m_markIcon_uncheck->show();
    }

	setCreateIcon(2);
    updateView();
    m_bMarked = bmarked;
}

多余的代码

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
// evb\src\gui\mgapollo\apps\tipdlg\src\MsgSelectListDlgExt.cpp
class MsgSelectListDlgExt : public PopClient, TimerEventListener
{
    public:
        static Controller* getInstance(void);
}
Controller* MsgSelectListDlgExt::getInstance(void)
{
    return m_instance;
}

// evb\src\gui\mgapollo\apps\common\src\AppWithBar.cpp
Controller* getMsgSelectListDlgInstance(void)
{
    return (MsgSelectListDlgExt::getInstance());
}

已经给了一个静态成员函数。使用类就可以调用了 Controller* instance = MsgSelectListDlgExt::getInstance();。所有 getMsgSelectListDlgInstance() 写得完全是多余。

写这个的人肯定也知道这个单例调用，但还是写了这个全局函数，不知道是故意的还是不小心的。

狗屎的命名方式

都 stop 了还要给个 bool 来设置开关状态。

1
void LcdService::stopService(NGBool openlcd)

多余的动作

这个函数是否背

重复调用 AudioPlayerService::isExistInstance()

重复获取实例。

1
2
3
4
5
6
7
8
9
10
11
// evb/src/gui/mgapollo/apps/audioplayer/src/AudioPlayerService.cpp

NGBool AudioPlayerService::IsBackgroudPlayingForOtherApp2()
{
    if (AudioPlayerService::isExistInstance())
    {
        AudioPlayerService* service = AudioPlayerService::getInstance();
        return service->IsBackgroundPlayingOn() && MediaManage::self()->IsCurMusicPlaying();
    }
    return false;
}

代码重复

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
// evb/src/gui/mgapollo/apps/audioplayer/src/AudioPlayerService.cpp

void AudioPlayerService::createRandomSequence(void)
{
    NGUInt index, i;
    NGUInt ran = 0, seed;

    // 获取系统时间作为种子
    seed = NguxTime::getSysTimeInSec();

    // 遍历索引序列
    for (index = 0; index < m_sequenceSize - 1;)
    {
        // 生成随机数
        ran = ((seed >> 3) ^ seed) & 0x1ffffffe;
        seed = (seed >> 29) | ran << 2;
        ran = ((seed >> 3) ^ seed) & 0xf;
        seed >>= 3;
        seed |= ran << 28;
        ran = (seed % (m_sequenceSize - index)) + index;

        // 检查生成的随机数是否合法
        if (ran >= m_sequenceSize)
        {
            // 打印调试日志。
            raw_uart_log("___AudioPlayerService:size %d,i %d,ran %d\n", m_sequenceSize, i, ran);
            
            // 重新生成种子。
            seed = NguxTime::getSysTimeInSec();

            // 进行
            continue;
        }

        // 。。。
    }
}

这里当索引值出错，返回的是索引值为 0 的文件名称。这里会导致一些问题。应该直接返回空字符串，或 NULL。并日志提示。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
// evb/src/gui/mgapollo/apps/filemanager/src/FileManager.cpp

const char* FileManager::GetCurItemName(void)
{
    // 如果存储列表可见
    if (m_storage_list->isVisible())
    {
		// 获取高亮项索引
        int index = (int)m_storage_list->hilightItemIndex();
		// 如果索引不在有效范围内
        if (index < 0 || index > 1)
        {
			// 设置索引为0
            index = 0;
        }

		// 返回存储项名称
        return arr_storage[index];
    }
}

重复代码

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
bool AppManager::moveApp2Top(NGCPStr name)
{
    BaseApp* app = NULL;
    AppInfo* curApp = NULL;

    if (NULL == (app = getAppByName(name)))
    {
        return false;
    }

    if (NULL != (curApp = m_appstack.top())
        && curApp->getName() == name)
    {
        return true;
    }

    if (curApp->getApp()->state() == BaseApp::RUNNING)
    {
        curApp->getApp()->setState(BaseApp::SLEEP);
        curApp->getApp()->onSleep();
    }

    if (m_appstack.move2Top(app))
    {
        app->onMove2Top();

        if (app->state() == BaseApp::SLEEP)
        {
            app->setState(BaseApp::RUNNING);
            app->onWakeup();
        }

        return true;
    }
    return false;
}

bool AppManager::moveApp2Top(BaseApp* app)
{
    AppInfo* curApp = NULL;

    if (NULL != (curApp = m_appstack.top())
        && curApp->getApp() == app)
    {
        return true;
    }

    if (curApp->getApp()->state() == BaseApp::RUNNING)
    {
        curApp->getApp()->setState(BaseApp::SLEEP);
        curApp->getApp()->onSleep();
    }

    if (m_appstack.move2Top(app))
    {
        app->onMove2Top();

        if (app->state() == BaseApp::SLEEP)
        {
            app->setState(BaseApp::RUNNING);
            app->onWakeup();
        }

        return true;
    }
    return false;
}

考虑不清楚

多选，只有两种状态，一个是全选，一个是没有选中。没有第三种状态。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
        typedef enum _MARK_MODE {
            MARK_NONE = 0,
            MARK_ALL = 1,
        }MARK_MODE;

/* evb/src/gui/mgapollo/apps/phonebook/src/PhoneBookList.cpp */

                    // if(m_markMode != MARK_NONE)
                    {
                        moveSelect();
                        AppManager *am = AppManager::getInstance();
                        BaseApp *currApp = am->getCurrentApp();
                        BaseApp *phbApp = am->getAppByName("phonebook");
                        if (currApp != phbApp) {
                            m_copyinterrupt = 1;
                            UILOGD("%s%d: current app has been destroy!\n", __func__, __LINE__);
                            return 0;
                        }
                        changeMarkMode(false);
                        loadContact();
                        m_list->hilight(0);
                        m_list->updateListView();
                        setTitle();
                    }

分支逻辑层级不一样

下面 if 判断的不是同一层级的东西。第一个是判断短信是否排序。第二个判断是否有 SIM 卡被初始化。

这种情况还是建议分开写。要么用 return 处理。

这里还有个问题。为什么会有 MMISMS_IsOrderOk 和 MMISMS_IsMMSOrderReady 两个函数。ok 和 ready 有什么区别么。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
/*****************************************************************************/
//     Description : to open the SMS main menu window。
//    Global resource dependence : 
//  Author: <USER>
//    Note: 
/*****************************************************************************/
void MMIAPISMS_OpenMainMenuWin(void)
{
    BOOLEAN is_not_inited = FALSE;

    // 检查是否有 SIM 卡未被初始化。
    is_not_inited = MMISMS_IsSimNotInited();

    // 
    if(MMISMS_IsOrderOk() && MMISMS_IsMMSOrderReady()) // 增加判断mms是否排序完毕
    {
        MMIAPISMS_ExitFromSMS();
        MMIAPISET_ValidatePrivacyAppEntry(MMISET_PROTECT_SMS, MMISMS_OpenMainMenuWindowExt);
    }
    else if(is_not_inited)
    {
        MMIPUB_OpenAlertWarningWin(TXT_SIM_NOT_INIT);
    }
    else
    {
        // 需要开启等待窗口    
        MMISMS_OpenInitWaitingWindow(MMISMS_OPEN_MAIN_MENU);
    }
}

魔法数字

根本不知道这个判断的是个什么东西，可以看得出判断的是数据类型，或数据类别。但根本不知道是什么类别。一旦修改业务或枚举值，这代码全部作废。

1
if (pPack->iDataType == 0 || pPack->iDataType == 5)