chanong 他留下了很多烂摊子，并面临着堆积如山的代码。 这时候我们可能会面临一个问题，我们是否应该重构？
大多数情况下，如果代码可以工作，我们尽量不去碰它，但如果必须重构它，我们该怎么办？ 重构的原则和技巧是什么？ 下面跟大家分享一下。
具体重构方法请参考《代码百科2》或《重构：改进现有代码的设计》。 本文不再面面俱到，而是重点讨论重构时一些粗浅的“方法论”，旨在提高重构的效率。
作者没有使用重量级的重构工具，只使用了“Smart”功能。 不使用CUnit等单元测试工具，而是通过在线调试和自动化测试来保证代码的正确性。
背景
MDU系列产品是从别人那里接过来的，包括笔者在内，与OMCI模块相关的只有三五个人。 除了开发新功能之外，还花费大量时间处理遗留故障。 但该模块代码复杂，可读性较差。 导致大家只了解它的“大概轮廓”，很难放心地使用和维护。
此外，忙碌也很容易导致迷失方向。 当主要时间和精力都花在故障排除上时，自然没有时间考虑整改代码，从而陷入到处救火、到处乱跑的尴尬境地。
两个进球
重构的主要目的是改进现有代码的设计，而不是修改缺陷、添加新功能等。
重构可以是简单的物理重构，如修改变量名、重新排列目录等，也可以是稍微复杂的逻辑重构，如提取子功能、精简冗余设计等。 但它们都没有改变现有代码的功能。
重构可以将意大利面条般的代码变成干净的、分层的代码。 干净的代码更加健壮，因为它可以轻松构建完整的测试围栏。 同时，无论是新手还是资深人士都可以放心地进行修改。
预计重构后，代码逻辑一目了然，扩展和修改会非常方便，故障能够快速定位和修复。 前人跌倒的地方，后人就不会再跌倒，前人的思考成果可以直接被后人借鉴。 总之，人性化程度高，极大地解放人力、脑力。
最初的想法是通过重构一些流程和代码（代码优先）来建立一个测试保护系统，生成阶段报告，并展示代码质量（示例加数据）和故障收敛曲线。 借助这样的报告，有望得到领导的支持和宣传，也有利于绩效考核。
三种做法
在具体实践中，作者并没有进行纯粹的“重构”，而是修改了缺陷，增加了自动化测试等辅助功能。 原则上，重点关注重构现有代码和重用新代码。
3.1 代码研究
OMCI模块代码复杂，分支多，上手难度大（据说需要半年上手，一年才能熟练）。 如果你不能有效掌握现有的代码，将来必然会被迫花费时间和健康却得不到项目的认可（事实上，模块中会源源不断地发现遗留故障）。 相反，如果你能够完全掌握现有的代码，那么你就可以通过逆向工程、系统/代码恢复和重构，使模块更容易开发和维护，最终解放编码人员自己。
为了提高代码阅读的效率，可以采用分块阅读和代码注释的方法。
“分读”是指将模块划分为若干子功能（如协议分析、报警、统计、二层、语音等）。 小组中每个人负责一个或几个区块，不时沟通、轮流。
“代码注释”是指在学习代码的过程中，可以对代码进行注释（大到一个流程、一个函数，小到一行代码），包括功能、意图、技巧、缺陷、问题、等等（有什么想到的都可以补充。注释）。 这些“问题”可以由兄弟产品同一模块的同事查阅，然后转化为功能或意图，也可以由其他注释者来回答。
这样做的优点是：避免重复研究； 经验的积累； 和量化。
代码可用产品最新版本，并建立服务器公共代码目录（SVN管理更好）。 只是评论时不要覆盖其他人的评论。
建议注释采用统一格式，方便识别和检索，如“//>”。 代码注释示例如下所示：
case OMCI_ME_ATTRIBUTE_2: // Operational state
if (attr.attr.ucOperationState != 0 && attr.attr.ucAdminState != 1) //xywang0618> BUG: should be ucOperationState!
{
return OMCI_FUNC_RETURN_OUT_OF_RANGE;
}
break;
3.2 可读性
首先，规范变量、函数等的命名，具体方法不再赘述。
其次，注释到位，特别是全局变量和通用函数。 示例如下：
/******************************************************************************
* 函数名称: ByteArray2StrSeq
* 功能说明: 掩码字节数组字符串化
该数组元素为掩码字节，将其所有值为1的比特位置转换为指定格式的字符串
* 输入参数: pucByteArray: 掩码字节数组
ucByteNum : 掩码字节数组待转换的有效字节数目
ucBaseVal : 掩码字符串起始字节对应的数值
* 输出参数: pStrSeq ：掩码字符串，以','、'-'间隔
形如0xD7(0b'11010111) ---> "0-1,3,5-7"
* 返 回 值: pStr ：pStrSeq的指针备份，可用于strlen等链式表达式
* 用法示例: INT8U aucByteArray[8] = {0xD7, 0x8F, 0xF5, 0x73};
CHAR szSeq[64] = {0};
ByteArray2StrSeq(aucByteArray, 4, 0, szSeq);
----> "0-1,3,5-8,12-19,21,23,25-27,30-31"
memset(szSeq, 0, sizeof(szSeq));
ByteArray2StrSeq(aucByteArray, 4, 1, szSeq);
----> "1-2,4,6-9,13-20,22,24,26-28,31-32"
* 注意事项: 因本函数内含strcat，故调用前应按需初始化pStrSeq
******************************************************************************/
CHAR *ByteArray2StrSeq(INT8U *pucByteArray, INT8U ucByteNum, INT8U ucBaseVal, CHAR *pStrSeq);
最后，修复晦涩难懂的代码。 主要有两种方法：
1）重写方法
以PON光路检测为例，底层接口提供的光功率单位为0.1uW，OMCI协议Test消息中上报的光功率单位为0.，Ani-G功率属性单位为0。
原代码转换如下（为了突出重点进行了调整）：
INT16S wRxPower = GetRxPowerInDot1uW(); //接收光功率
if(wRxPower < 1){
wRxPower = 1;
}
/*0.1uw to 0.002dbm*/
dblVal = 10 * log10(wRxPower) - 40;
dblVal = dblVal * 500;
wRxPower = (INT16U)dblVal;
wRxPower = (int)wRxPower*100;
/*opt pwr 0.00002db X * 0.00002*/
wRxPower = wRxPower + (30 * 500) * 100;
if(wRxPower < 0){
val = (INT16U)((0 - wRxPower) / 100);
val = (((~val) & 0x7fff) + 1) | 0x8000;
wRxPower = val;
}
else{
wRxPower = wRxPower / 100;
}
可见，原来的实现中的转换关系非常晦涩难懂。 其实借助1dBuW=10*lg(1uW)和1dBuW-1dBmW=30dB这两个公式，通过简单的数学推导就可以得到更加简洁易懂的表达方式（适配突出重点） ：
INT16S wRxPower = GetRxPowerInDot1uW(); //接收光功率
//Test单位0.002dBuW，底层单位0.1uW，转换关系T=(10*lg(B*0.1))/0.002=5000*(lgB-1)
wRxPower = (INT16S)(5000 * (log10((DOUBLE)wRxPower)-1));
//Ani-G功率属性单位0.002dBmW，Test结果单位0.002dBuW
//转换关系A(dBmW)*0.002 + 30 = T(dBuW)*0.002，即A=T-15000
INT16S wAniRxPwr = wRxPower - 15000;
请注意，最初的实现错误地认为 Ani-G 功率属性与测试结果的单位相同。 这个错误已在新的实现中得到纠正。
2）封装功能
以实体属性的mask验证为例，原代码如下：
/*掩码初校验*/
if ((OMCIMETYPE_SET == vpIn->omci_header.ucmsgtype)
|| (OMCIMETYPE_GET == vpIn->omci_header.ucmsgtype))
{
wMask = W(response.omcimsg.auccontent[0],response.omcimsg.auccontent[1]);
usSupportMask = (1 << (OMCI_ATTRIBUTE_NUMBER - map.num))-1;
if( 0 != (wMask & usSupportMask))
{
OmciPrint_warn("[%s] check mask warning: (meclass[%u], meid[%u], msgtype[%u], mask[0x%x], unsupport mask[0x%x])!\n\r",
FUNCTION_NAME, vpIn->omci_header.wmeclass, vpIn->omci_header.wmeid, vpIn->omci_header.ucmsgtype, wMask, usSupportMask);
}
}
赋值和判断语句（第6至7行）用于检查掩码是否越界。 为了提高可读性，将其封装成以下函数：
/******************************************************************************
* 函数名称: OmciIsMaskOutOfLimit
* 功能说明: 判断实体属性掩码是否越界(比特1数目超过属性数目)
* 输入参数: INT16U wMeMask :实体掩码
* INT8U ucAttrNum :属性数目
* 输出参数: NA
* 返 回 值: BOOL
******************************************************************************/
BOOL OmciIsMaskOutOfLimit(INT16U wMeMask, INT8U ucAttrNum)
{
//wMeMask :mmmm mmmm mmm0 m000
//wInvertMask :0000 0000 000i iiii
INT8U wInvertMask = (1 << (OMCI_ATTR_MAX_NUM-ucAttrNum)) - 1;
return (0 != (wMeMask & wInvertMask));
}
封装的函数名称正确地充当“自描述”函数。
3.3 在线调试项目
该产品作为嵌入式终端，需要在Linux系统中编译打包版本，然后下载到目标板运行。 这种交叉编译方式对于调试单个模块来说无疑是低效的。
为了提高调试效率，在Linux服务器上搭建了在线调试项目。 即提取OMCI模块代码，稍作修改后直接在服务器上编译运行。 这样就避免了每次修改代码时都必须重新启动开发板来升级主要版本，从而使调试变得极其高效。
为了让模块独立运行，需要编写模拟接口，屏蔽底层调用，剪掉暂时不需要的功能（比如线程、通信）。
3.4 模拟数据库
OMCI模块使用内存数据库来管理需要持久化的实体信息，但数据库代码调用了大量平台相关的接口，不利于模块的在线调试。 因此，作者研究了源码，写了一个模拟数据库。 该库模仿了模块使用的几个原始库接口和行为。 模拟接口内部验证增加错误信息打印，方便排查问题。
另外，基于数据库接口原语对统一接口进行了两次封装，一举消除了模块中数据库操作代码的混乱和重复。
3.5 自动化测试
不检测防护网的重建，无异于没有血源的手术。
首先，公共接口和函数都提供相应的测试函数，作为示例和用例。 喜欢：
//Start of ByteArray2StrSeqTest//
VOID ByteArray2StrSeqTest(VOID)
{
//ByteArray2StrSeq函数算法不甚优美和严谨，应多加测试验证，如有可能尽量优化。
INT8U ucTestIndex = 1;
INT8U pucByteArray[] = {0xD7, 0x8F, 0xF5, 0x73, 0xB7, 0xF0, 0x00, 0xE8, 0x2C, 0x3B};
CHAR pStrSeq[50] = {0};
//Time Consumed(x86_gcc3.2.3_glibc2.2.5): 72us
memset(pStrSeq, 0, sizeof(pStrSeq));
ByteArray2StrSeq(pucByteArray, 4, 1, pStrSeq);
printf("[%s] Result: %s, pStrSeq = %s!\n" , __FUNCTION__, ucTestIndex++,
strcmp(pStrSeq, "1-2,4,6-9,13-20,22,24,26-28,31-32") ? "ERROR" : "OK", pStrSeq);
//Time Consumed(x86_gcc3.2.3_glibc2.2.5): 7us
memset(pStrSeq, 0, sizeof(pStrSeq));
ByteArray2StrSeq(pucByteArray, 4, 0, pStrSeq);
printf("[%s] Result: %s, pStrSeq = %s!!!\n" , __FUNCTION__, ucTestIndex++,
strcmp(pStrSeq, "0-1,3,5-8,12-19,21,23,25-27,30-31") ? "ERROR" : "OK", pStrSeq);
//Time Consumed(x86_gcc3.2.3_glibc2.2.5): 4us
memset(pStrSeq, 0, sizeof(pStrSeq));
ByteArray2StrSeq(&pucByteArray[4], 2, 1, pStrSeq);
printf("[%s] Result: %s, pStrSeq = %s!\n" , __FUNCTION__, ucTestIndex++,
strcmp(pStrSeq, "1,3-4,6-12") ? "ERROR" : "OK", pStrSeq);
//Time Consumed(x86_gcc3.2.3_glibc2.2.5): 4us
memset(pStrSeq, 0, sizeof(pStrSeq));
ByteArray2StrSeq(&pucByteArray[6], 2, 1, pStrSeq);
printf("[%s] Result: %s, pStrSeq = %s!\n" , __FUNCTION__, ucTestIndex++,
strcmp(pStrSeq, "9-11,13") ? "ERROR" : "OK", pStrSeq);
//Time Consumed(x86_gcc3.2.3_glibc2.2.5): 5us
memset(pStrSeq, 0, sizeof(pStrSeq));
ByteArray2StrSeq(&pucByteArray[8], 2, 1, pStrSeq);
printf("[%s] Result: %s, pStrSeq = %s!\n" , __FUNCTION__, ucTestIndex++,
strcmp(pStrSeq, "3,5-6,11-13,15-16") ? "ERROR" : "OK", pStrSeq);
}
//End of ByteArray2StrSeqTest//
此外，模块中还添加了自动化测试功能（），可用于验证批量或单个实体的配置和查询操作。 批次测试结果统计如下（各实体具体测试结果略）：
上述测试结果中，(s)最为关键，表示失败用例的数量。 另外，(s)表示尚未比较的项目数。 例如，获取时间等可变属性的实体无法预设适当的预期结果，因此不进行比较。 测试过程中的打印信息可以保存为日志文件，然后通过在打印日志中搜索关键字，可以了解哪些配置失败。
当大量当前代码被修改时，借助上述自动化测试功能可以快速得知修改结果的影响。 开发新功能时，可以先设计测试用例和预期结果，然后按照“测试驱动开发”模型进行编码，提高编码效率和准确性。
3.6 深入核心
传统的重构步骤是先易后难，先外围后核心。 作者反其道而行之，首先重构了核心公共代码。 这样做的好处是：
1）方便整理头文件包含关系
在线调试项目中，最初只保留最常用的代码文件（如日志功能）。 经过重构和调试，逐步添加其他单一功能的目标代码。 此过程根据需要分割和/或组合文件，减少头文件的嵌套和交叉引用。
2）避免重复工作甚至返工
公共代码经过重构和封装后，在重构外围应用代码时会更容易消除冗余。 如果你先重构外围代码，你很可能会发现一些逻辑可以统一到公共代码中，从而导致大量返工； 而如果你先开始重构公共代码，通过研究外围代码如何使用它，那么早做起来就会很容易。 筛选这些裁员。
3）迭代验证
当外围代码逐渐叠加到重构后的公共代码上时，公共代码的正确性和易用性也得到了反复的检验。
4）增强信心
先核心后外围、逐步叠加验证的过程是可控的，可以在大规模重构时增强信心、缓解压力。 相反，如果先重构外围代码，触及核心，就会牵一发而动全身，压力会极大。
四大功效
基于某产品代码，重构OMCI模块DB/LOG/实体访问/消息处理/性能统计等。经过三个多月的重构，模块代码复杂度明显下降（某核心源码平均复杂度文件下降到原来的1/4），代码大幅精简（据不完全统计，精简了一万多行），可读性更强。 在添加新代码的过程中，编写了大量的工具宏和函数，并添加了OMCI自动化测试、内存检测等实用功能。
通过并衡量某功能代码的重构效果，如下表所示：
此外，重构过程中积累的通用框架、代码和经验可以进一步应用到新项目中。
原来的：
微信扫一扫打赏
支付宝扫一扫打赏
