C++老代码重构实战：安全网、坏味道与智能指针的落地路径

我刚接手那个模块的时候第一反应是“这代码能跑真是运气好”。一个函数里塞了订单校验、价格计算、折扣规则、库存扣减、日志和通知两百多行从头到尾只有三个空行魔法数字散落在好几个文件里裸指针用得到处都是有人忘了delete有人删了两次。可就是这样一坨代码没人敢动它因为一动就崩一崩就要花几个晚上查。后来我花了两周时间把它一点点拆开、理顺、补上测试整个过程总结下来就六个字小步、守旧、增量。这篇文章就是把那两周踩过的坑和沉淀下来的C代码重构技巧写出来包括重构前必须搭好的安全网、老代码里最常见的坏味道、拆“怪物函数”和拆继承体系的真实过程以及翻车时怎么把自己捞回来。如果你手头也有一个能跑但不敢改的C老模块这篇应该值得你花十分钟读完。1. 重构前先搭好三张安全网再谈技巧1.1 没有测试开路的重构等于裸奔重构的底线是什么业界有个共识重构是在不改变外部行为的前提下改善内部结构。这句话的每一个字都重要尤其是“不改变外部行为”。可问题在于你怎么知道自己有没有改变外部行为靠眼睛盯靠脑子记都不靠谱。我见过最典型的翻车现场某同事重构一个工具函数自认为“只是把变量名改清楚了一点”结果把一个边界条件从改成了整个系统的配额判断全部偏了一位。那个问题在预发环境躺了三天才被发现期间数据已经错了一大片。所以重构的第一步不是打开编辑器而是先搭好安全网。安全网最基础的一张就是测试。说实话动手重构前给老代码补测试是一件又枯燥又容易让人怀疑人生的事情但它真的能救命。你不需要覆盖率100%但凡是接下来要动的函数、要拆的类、要改的调用关系全都要有可运行、可断言、可重复的用例。测试的粒度也值得说两句。不要一上来就写一大坨端到端测试定位问题太困难。尽量对着函数签名写单元测试输入输出都给清楚函数依赖太多的先用接口替身把它孤立出来。等测试亮了绿灯你才真正有了“行为不变”的裁判。1.2 建立编译基线与警告清零第二张安全网是编译。听起来像废话但很多老项目连“干净编译”都做不到。我接手那个模块时打开编译日志几百条警告有未使用变量、有隐式类型转换、有废弃API调用甚至有两个名字一模一样的函数在不同的头文件里声明。这种状态下做重构你根本分不清新问题是自己改出来的还是原本就有的。所以动手之前我会花半天时间把编译警告清零。办法不复杂先打开-Wall -Wextra一条一条过。能修的修确认无害的用局部抑制并写注释说明原因。等编译器一个警告都不报了再做一次基线提交。之后每次重构编译器的输出就是一面镜子如果改动之后还有任何警告那基本可以断定是我这次改出来的必须当场解决。这里有个小技巧值得分享把编译命令写成一个脚本固定下来连同编译选项一起提交进仓库。很多老项目能编过全靠某台机器上的环境变量换台电脑就编不过。把基线固化下来后面所有人包括未来的自己都在同一套标准上动手。1.3 给老模块补上“行为快照”除了测试和编译我还会给老模块做一层“行为快照”。说白了就是在重构前把模块处理过的典型输入、边界输入、异常输入全部跑一遍把输出记录下来存成一组静态的“正确结果”。这个做法在重构带IO的模块时特别好用。比如某个日志解析器输入一批日志文件输出结构化数据。重构前跑一遍把输出文件存成基线重构后跑一遍对比差异。只要差异为空说明行为保持住了。有人管这叫黄金文件测试也有人管这叫快照测试原理都一样。对于纯函数逻辑行为快照更简单写一段临时代码把输入和输出打出来保存成日志之后做对比。目的就一个——在测试覆盖不到的地方用最朴素的手段接住行为变化。等你把模块拆完了再把这些快照里容易回归的部分转换成正式用例补进测试套件里。到这里三张网成形测试兜底逻辑编译兜底语法和类型快照兜底那些测试还没来得及照顾到的行为。有了这三样你才总算可以打开编辑器开始重构了。2. 老代码里最典型的八种C坏味道以及对症的改造思路2.1 先说一句大实话坏味道不等于错误在聊坏味道之前我得先给读者提个醒坏味道不是bug不是说代码这么写就一定会出问题。它更像一种“未来会出问题的倾向”。一个带魔法数字的常量不一定错一个200行的函数也不一定崩但它们都会让下一次需求变更变得特别痛苦。所以下面这份清单是我自己在评估“要不要重构”时的参考不是检查清单不用全中才动手。常见且让我看一眼就想重构的C代码我大致列成一张表坏味道典型症状远期后果常用的重构手法魔法数字if (x 3)、sleep(5000)遍地都是改规则时漏改一处行为不一致提取命名常量constexpr巨型函数超过一两百行一个函数干五六件事没法复用、没法测试、改一处崩三处提取函数Extract Function裸指针满天飞new/delete散落在业务代码里内存泄漏、重复释放、所有权混乱RAII、std::unique_ptrconst 缺失参数、成员函数全部不带 const接口语义模糊误改共享状态补 const 限定符和引用拷贝控制缺失类里含有指针却不定义析构、拷贝、移动浅拷贝、双重释放Rule of Five / Rule of Zero头文件依赖混乱一个头文件 include 十几个头文件编译时间爆炸依赖隐藏Pimpl 惯用法、前置声明继承体系过深类层次五六层虚函数层层覆盖脆弱基类改底层影响一片组合优先、策略对象全局可变状态一堆全局变量或单例互相读写顺序敏感、难测试、并发翻车依赖注入、显式状态传递2.2 为什么“魔法数字”值得被认真对待魔法数字是最不起眼但最阴险的坏味道。表面上看把0.85换成kVipRate好像只是换了个名字没有任何行为变化。但真正的问题不在名字而在于“魔法数字散落多处”会让约束失联。我之前维护过一个计费模块折扣率在不同文件里出现了五六次有写0.85的有写85 / 100.0的还有一个地方写的是1 - 0.15。表面看都是同一个折扣但后来业务政策调整需要区分“常规折扣”和“活动折扣”这几个数字各自要改成不同的值。由于它们之间没有任何关联改起来就是纯靠搜索漏一个就出事。重构的方法不复杂把每个含义明确的常量提取成具名的constexpr放进它所属的语义单元里。比如namespace discounts { constexpr double kVipRate 0.85; }。提取之后再跑测试你甚至能发现以前“碰巧一致”、实际上语义根本不同的地方。这一步几乎零风险但收益非常大。2.3 拷贝控制缺失C特有的隐形地雷有些坏味道不是C也基本遇不到比如“类里放了指针却没有正确处理拷贝控制”。这是我眼里优先级最高的一类问题因为它往往不炸一炸就是线上事故。你可能会写这样的类class Buffer { public: Buffer(const char* data, size_t len) { data_ new char[len]; std::memcpy(data_, data, len); len_ len; } ~Buffer() { delete[] data_; } char* data_ nullptr; size_t len_ 0; };这个类能用吗能。但它只要被复制一次——哪怕是通过函数参数传值进来——就会发生浅拷贝两个对象指向同一块内存析构时双重释放或者任意一个对象修改了内容另一个莫名其妙跟着变。这种问题怎么重构两个方向。一个是遵循现代C的建议让类里尽量不要出现裸管理职责把资源交给标准库容器比如std::vectorchar。这就是“Rule of Zero”——你不写析构、拷贝、移动编译器生成的默认版本就是对的。另一个方向是如果确实需要自己管理资源就完整实现“Rule of Five”析构、拷贝构造、拷贝赋值、移动构造、移动赋值缺一不可。说句实在话我在实际重构里见过的大部分Buffer、Connection、ResourceHandle这类类改成std::vector或std::unique_ptr之后代码量反而更短析构拷贝移动五个函数全都可以删掉。能删代码的重构是最有成就感的重构。3. 一次真实的“怪物函数”拆解从200行到5个小函数的完整路径3.1 那个让人不敢动的“订单处理”函数下面这个案例是我实际工作中某个类似场景的简化版。某公司内部有个订单服务核心入口是一个叫handle的函数当时大概200行。我接手时光是这个函数的缩进深度和圈复杂度就已经够劝退人了。为了讲清楚我把它精炼成下面这个样子——做的事情完全一样只是省掉了各种业务细节void OrderService::handle(Order order) { if (order.getItems().empty()) { throw std::invalid_argument(empty order); } for (const auto item : order.getItems()) { if (item.getQty() 0 || item.getPrice() 0) { throw std::invalid_argument(invalid item); } } double total 0.0; for (const auto item : order.getItems()) { total item.getPrice() * item.getQty(); } if (order.isVip() total 1000.0) { total * 0.85; } order.setTotal(total); for (const auto item : order.getItems()) { inventory_.deduct(item.getSku(), item.getQty()); } logger_.info(order processed, id order.getId()); if (order.getEmail().empty() false) { mailer_.sendReceipt(order); } }看着也就四五十行实际工程里比这夸张的多的是。但已经能看出问题校验、算总价、折扣、扣库存、记日志、发邮件六件事挤在一个函数里。任何一个环节想做调整都必须先读懂全部上下文测试也只能端到端一把抓。3.2 拆解第一步先做小块提取不做大手术重构这种巨型函数最关键的原则是“小块提取、安全落地”一次只从原函数里抽出一小块代码移动到独立的函数中然后立刻编译、跑测试。不要试图一次性设计出最终形态那是赌徒心态。我第一步抽的是校验逻辑。把它整块拿出来变成OrderService里的一个私有静态函数bool OrderService::validate(const Order order) { if (order.getItems().empty()) { throw std::invalid_argument(empty order); } for (const auto item : order.getItems()) { if (item.getQty() 0 || item.getPrice() 0) { throw std::invalid_argument(invalid item); } } return true; }原函数里那一大段if逻辑就替换成一行validate(order);。注意这里我严格保持了异常类型不变、抛出条件不变、参数也尽量用const引用。因为一旦抛出的异常类型变了外部捕获逻辑的行为就会跟着变这就违反了“外部行为不变”。第二步抽价格计算。这里是纯计算逻辑不依赖任何成员状态所以我把它也做成静态函数并让computeTotal和applyDiscount分离double OrderService::computeTotal(const Order order) { double total 0.0; for (const auto item : order.getItems()) { total item.getPrice() * item.getQty(); } return total; } double OrderService::applyDiscount(double total, const Order order) { if (order.isVip() total 1000.0) { return total * 0.85; } return total; }注意这里我还没有动魔法数字0.85和1000.0。按我的原则重构和改业务规则必须分开。0.85要改成具名常量这件事我会单独做会单独提交绝不和拆函数混在一起。3.3 拆解第二步参数与状态整理函数拆到一定程度你会发现有些逻辑天然属于同一组数据。比如扣库存和邮件通知都依赖“订单里的条目”但它们关心的字段完全不同。这时候不用硬塞进同一个函数保持独立即可。扣库存的逻辑依赖inventory_所以它更适合做成员函数void OrderService::deductStock(const Order order) { for (const auto item : order.getItems()) { inventory_.deduct(item.getSku(), item.getQty()); } }日志和通知同理各归各的void OrderService::recordProcessLog(const Order order) { logger_.info(order processed, id order.getId()); } void OrderService::sendReceipt(const Order order) { if (order.getEmail().empty() false) { mailer_.sendReceipt(order); } }到这里每个函数做的事情都短到可以在一屏内看完整。测试可以分别针对computeTotal、applyDiscount、validate写用例根本不用再构造一个复杂到家的订单来触发所有分支。3.4 拆完之后的整体形态最后handle函数变成了一串意图清晰的调用void OrderService::handle(Order order) { validate(order); double total applyDiscount(computeTotal(order), order); order.setTotal(total); deductStock(order); recordProcessLog(order); sendReceipt(order); }这段重构成交的点有几个单函数缩进层级明显下降、每个被拆出来的函数都有独立测试入口、后续改动“校验规则”或“折扣规则”时不需要再翻阅整个200行大函数。经历过一次这样的拆解之后回头再看原来的代码我真的想不通当时是怎么在那种结构里加需求的。4. 裸指针缠身的老模块RAII与智能指针的无痛改造4.1 裸指针代码的典型症状C老代码里最常见的另一个重灾区就是裸指针。我处理过一个图像缓存模块它用裸指针管理一大块图像数据里面有缓存成员、有临时对象、还有函数返回裸指针让外部释放。典型的症状是内存泄漏的bug查不清、偶发的崩溃经过很久才发现是双重释放、每个拿到指针的人都想知道“这个到底归不归我释放”。看一段浓缩版的原代码Image* ImageLoader::load(const std::string path) { if (cache_ ! nullptr) { return cache_; } Image* img new Image(path); cache_ img; return img; } void ImageLoader::reset() { delete cache_; cache_ nullptr; }这段代码的问题一眼就能看出来如果外部拿到cache_指针后没有置空就调用reset()外部指针就悬空了如果外部自己delete了一份reset()又删一次直接双重释放。最要命的是函数返回裸指针等于把“所有权到底归谁”这个问题丢给了调用方而调用方每次都猜。4.2 第一步消灭裸new交给unique_ptr这种模块的重构第一步是大量消除裸new。把缓存成员从Image*改成std::unique_ptrImage加载函数改为返回const Imageclass ImageLoader { public: const Image load(const std::string path); void reset(); private: std::unique_ptrImage cache_; }; const Image ImageLoader::load(const std::string path) { if (!cache_) { cache_ std::make_uniqueImage(path); } return *cache_; } void ImageLoader::reset() { cache_.reset(); }原来需要手动delete的地方全部消失因为unique_ptr在析构时会自动释放。还有一个关键变化原来的load返回裸指针意味着调用方可以对缓存对象动手脚现在返回const Image从类型上宣布“缓存共享可读所有权归ImageLoader”。如果真的有调用方需要一份独立的图像对象那应该让load返回std::unique_ptrImage内部拷贝一份再移交——这两个语义现在分得清清楚楚。4.3 第二步处理异常路径和共享所有权裸指针重构里还有一个容易被忽略的地方是“可能中途返回的函数”。看这个旧代码Resource* acquire() { Resource* r new Resource(); if (!init(r)) { return nullptr; // 泄漏没人释放r } if (!prepare(r)) { return r; // 调用方拿到后要记得释放 } return r; }这种函数放在智能指针时代应该是这样std::unique_ptrResource acquire() { auto r std::make_uniqueResource(); if (!init(*r)) { return nullptr; } if (!prepare(*r)) { return r; } return r; }unique_ptr在作用域结束时会自动释放所以那些提前return的路径不会泄漏正常路径把所有权交出去释放责任跟着对象走。这是RAII最核心的价值把资源管理绑定到对象生命周期上而不是绑定到程序员的记忆上。至于shared_ptr我一般只在真正需要“多个持有者共同拥有同一个资源”的时候使用。比如多个子系统都要保存同一个配置对象的引用且任何一方都可能先于其他方析构。其余情况优先unique_ptr——它的性能开销更小语义也更清晰。用shared_ptr用得太多代码会变成“谁都不确定自己是不是最后一个释放者”生命周期反而变模糊。4.4 重构过程中的“谁拥有谁释放”自查清单这轮重构做下来我总结了一份自查清单每次碰到裸指针都会过一遍这个指针是观察还是有些owns对象观察用裸指针或引用所有用智能指针。如果所有是唯一所有还是共享所有唯一用unique_ptr共享用shared_ptr。函数返回值是借用还是转移借用返回引用转移返回unique_ptr。类的成员指针是否跨线程使用跨线程先别急着改智能指针先把线程模型理清楚再说。这份清单帮我挡掉了不少坑。特别是最后一条别在没搞清并发模型的情况下乱上智能指针否则浅层问题没了深层的数据竞争还在而且更难查。5. 让人头疼的继承螺旋组合优先原则的一次落地5.1 让我下决心拆掉那个继承体系的导火索第三个真实案例是某个报表导出模块。最初的类图看起来很漂亮一个基类ReportExporter下面挂着CsvExporter、JsonExporter、PdfExporter后来因为要支持不同页眉页脚样式又长出WithHeaderCsvExporter、WithHeaderJsonExporter、WithFooterCsvExporter…… 没过多久类层次已经到了五六层深虚函数覆盖和重载标记交叉混乱。压垮我的导火索是那次需求客户希望“CSV格式不要页眉但PDF格式要两个页脚”。按照继承体系要么再新建三个子类要么在虚函数里加好几个布尔开关。我打开那个文件看到抽象基类里已经躺着七八个virtual方法、三个默认参数、还有一个完全没人调用的遗留虚函数。我当时就决定这个继承螺旋必须拆。5.2 战略与组合的改造过程继承要描述的是“是一个”的关系但报表导出的变化维度其实是“输出格式”和“装饰样式”两件事它们互相组合。用继承硬来的话类数量就是笛卡尔积。我最后改成了组合加策略的模式。先把核心流程拆出来class ReportExporter { public: ReportExporter(std::functionstd::string(const Report) serializer, std::functionstd::string() header, std::functionstd::string() footer) : serializer_(std::move(serializer)), header_(std::move(header)), footer_(std::move(footer)) {} void exportTo(const Report report, std::ostream out) { out header_() serializer_(report) footer_(); } private: std::functionstd::string(const Report) serializer_; std::functionstd::string() header_; std::functionstd::string() footer_; };导出过程固定成三段写页眉、写正文序列化、写页脚。格式和样式的差异全部塞进std::function策略对象里通过构造函数在运行时注入ReportExporter makeCsvExporter(bool withHeader, bool withFooter) { std::functionstd::string() header withHeader ? std::functionstd::string()(makeCsvHeader) : std::functionstd::string()([] { return ; }); std::functionstd::string() footer withFooter ? std::functionstd::string()(makeCsvFooter) : std::functionstd::string()([] { return ; }); return ReportExporter(serializeCsv, header, footer); }这样想要“CSV不要页眉、PDF双页脚”就不再需要新建类只是两个策略对象的组合问题。新增一种输出格式也只需新增一个序列化函数和一个工厂函数不会影响任何已有策略。整个报表模块类层次从五六层压成了一层测试也容易得多每个策略函数单独可以测三个策略的各种组合也可以分别测。5.3 什么时候真的该保留继承我得承认组合优先不是“继承有罪”。如果继承体系真的稳定、层次浅、并且确实存在“是一个”的语义比如不同类型的日志处理器共享同一个基础流程那保留继承没有问题。我说的是那种为了复用几行代码硬造的父子关系或者是为了“看起来面向对象”而不断往下挂子类的设计。碰到这种组合基本是更优解。另一个实用判断标准如果你添加一个新特性需要修改基类或者需要新建两个以上带组合名字的子类那大概率是继承体系已经撑不住了。这时候与其继续挂类不如退一步想想哪些维度是独立的把每个维度变成策略对象然后用组合把它们拼起来。重构完成后你会发现新增需求的工作量从“改三层代码”变成了“加一个策略”。6. 重构最容易翻车的三个瞬间以及我怎么把车救回来的6.1 翻车瞬间一重构时顺手改了一个bug重构和修bug是两件事。行为保持是重构的底线而修bug本质上是改变行为。最忌讳的是我在拆解“订单处理”函数的时候看到一个item.getPrice() 0的校验心想“这不对啊应该是 0”然后顺手就改了。表面上只动了一个字符但这次提交既改了结构又改了规则万一后续出了问题根本没法定位到底是结构调整引起的还是规则变化引起的。我的救法很简单把这类“顺手发现的问题”记成清单等重构提交完成并且所有测试都通过之后再单独开一个提交去修。这样每个提交的意图都足够纯粹。评审的时候重构提交只需要看“结构变没变坏、行为有没有保持”bug修复提交只需要看“逻辑对不对”。谁都轻松。6.2 翻车瞬间二编译过了行为却变了比编译不过更可怕的是编译过了但行为变了。C有不少行为变化是编译器不报错的比如把int参数改成double之后隐式转换发生变化、把std::map换成std::unordered_map之后遍历顺序变了、把一个参数从“传值”改成“传引用”之后原本对副作用的依赖直接被打破。这种案例最需要行为快照和测试来兜底。我在一次重构里把某个模块的std::map换成了std::unordered_map编译全过、单测全绿但黄金文件对比出来上百行不一致——因为旧逻辑里隐含依赖了map的有序性。那一次要是没有做快照上线后肯定要出大事。所以我现在有个铁规矩凡是涉及容器、参数传递方式、运算顺序的重构必须跑行为快照对比不能只看测试绿不绿。6.3 翻车瞬间三把格式化和重构混在一个提交里还有一次翻车是纯自找的我重构一个类顺手把整个文件从Tab缩进换成了四个空格还用上了格式化工具自动整理。提交一出来diff 里一半是格式变化一半是逻辑变化评审的人根本没法看。几种意见上蹿下跳最后那一次提交被打了回来。从那以后我定了规矩格式化单独一个提交重构单独一个提交bug修复再单独一个提交。格式化提交可以完全不碰任何逻辑只做格式化这样它在 diff 里就很容易被认出来。如果评审工具支持优先用“忽略空白差异”的方式来看代码效果也很好。6.4 事后防回归最小规模的自动化支撑最后补一条经验重构之后一定要把防回归的自动化支撑立起来。不需要一开始就搞特别重的流水线先做到两件事。第一每次提交都自动跑一遍目标模块的单元测试第二在可控范围内让编译器把警告当错误处理也就是常说的-Werror。这两件做到了后面再往流水线里加静态分析、覆盖率统计都会很顺。我个人现在的习惯是重构代码的同时就顺手把对应的测试用例补充到测试套件里而不是等重构完再补。测试和重构同步走每一小步都很安全即便哪天我突然被调走去处理别的事同事接手也能顺着提交记录和测试用例快速理解意图。C重构这件事说到底拼的不是技巧多花哨而是谁更稳。把每一步都做成“可以回退、可以解释、有测试兜底”的提交日积月累再烂的模块也能被慢慢理顺。我在那两周时间里最重要的收获不是那些被拆出来的小函数而是这套“小步、守旧、增量”的工作方式——到现在我接手任何新模块都会先问自己一句这次重构我的安全网搭好了吗