minikv 重构项目 · 任务 5 / 8

m05:该合的合,该分的分

对应第 9 章(合在一起还是分开?)

背景与对应章节

server.cpp 里哪些代码应该合成一处?哪些本该分开的东西被搅在了一起?哪两个方法必须对照着读才看得懂?

Server 是 minikv 的命令层:把一行文本切成词,找到对应的处理函数,处理函数检查参数、调用 Database、格式化回复。每加一个命令,同事就复制一个现成的处理函数改一改;某天需要支持"值里带空格",又在切词函数里加了个特殊分支。代码一直能跑,测试也全绿。

这个任务练的是第 9 章的判断:两段代码共享信息、合起来能让接口更简单、或者合起来能消除重复时,应该合;通用机制和专用逻辑应该分开;而拆分本身也有代价。做之前请读 第 9 章 · 合在一起还是分开?,重点是原书 9.3(消除重复)、9.4~9.5(通用与专用分开)、9.7~9.8(方法的拆与合,以及和 Clean Code 的分歧)。

你要找的问题

这次只看两个文件:src/server.h 和 src/server.cpp(外加 Store::incr 里的一小段)。读的时候回答:

位置读的时候问自己
10 个 handleXxx() 的开头哪几行在每个函数里都出现?把它们排成一列对比:有没有哪一份和别的不一样?不一样是有意的,还是复制时漏了?
handleIncr()、handleGetRange()、Store::incr()三处都在把字符串解析成整数。它们对 12abc、0x 的处理一样吗?
Server::tokenize()一个"按空白切词"的函数,为什么要知道命令名?再加一个值里能带空格的命令时,你得记得改哪里?
handleSet() 的第一行SET 明明有键和值两个参数,为什么检查的是 args_.size() != 2?rawValue_ 是什么时候、被谁填的?不看别的方法,你能读懂这一行吗?

把找到的每一处问题记下来,标上它属于"重复""通用与专用混杂""连体方法"中的哪一种。

确认测试全绿,然后亲手验证一下副本之间的不一致:

cd project/minikv
make test
printf 'SET k hello\nGETRANGE k 0 1x\nINCR n 1x\n' | ./minikv /tmp/m05.log; rm -f /tmp/m05.log
grep -n "wrong number\|kMaxKeyLength\|stoll" src/server.cpp

任务与约束

  1. 消除重复:参数个数检查、键长检查、整数解析,各只保留一份。
  2. 让切词函数回到通用:它不应该知道任何命令名。"哪个命令的最后一个参数可以带空格"这件事,放到描述命令的地方去。
  3. 拆开连体方法:处理函数需要的输入通过参数传进来,不再通过成员变量和切词函数暗中交接。
  4. 如果修重复时顺手修掉了副本之间的不一致,导致某些输入的回复变了,把变化列出来,说明为什么新行为是对的。

约束:

自查清单

提示

提示 1:重复的部分其实是"数据"

把每个处理函数开头的检查排成表格:命令名、最少几个参数、最多几个参数、第一个参数是不是键。你会发现各个函数之间不同的只是这几个数,相同的是检查逻辑。数据放进一张表,逻辑写一遍。

提示 2:怎么让切词函数不认识 SET

很多语言的 split 都有一个"最多切几段"的参数:切到最后一段时,剩下的部分原样保留。这个参数是通用的,和命令无关。"SET 最多两个参数、最后一个吃掉整行剩余"可以写进 SET 的命令描述里,由分派代码把它翻译成这个参数。

提示 3:整数解析值得一个函数吗

第 9.3 节说,抽出重复代码只在两种情况下划算:被抽出的代码不算太短,且抽出后的签名简单。整数解析符合吗?想想 strtoll 的几个坑:前导空白、在第一个非数字字符处悄悄停下、溢出要看 errno。基线的副本各自踩了不同的坑。反过来,处理函数里那两三行 try/catch,抽成函数后签名会是什么样?

参考答案与讨论

先自己做完、测试全绿,再打开下面的内容。你的命令表字段、函数名和参考答案不同都没关系;要检查的是:每段知识是不是只出现一次,每个函数是不是能单独读懂。

打开参考答案

重复 → 一张命令表

std::string Server::handleGet() {
    if (args_.size() != 2) return "ERR wrong number of arguments for 'get'";
    const std::string& key = args_[1];
    if (key.size() > kMaxKeyLength) return "ERR key too long";
    ...
}
std::string Server::handleAppend() {
    if (args_.size() != 2 || rawValue_.empty()) return "ERR wrong number ...";
    const std::string& key = args_[1];
    // (键长检查漏掉了)
    ...
}
std::string Server::handleGetRange() {
    ...
    start = std::stoll(args_[2]);   // "1x" 被当成 1
    end = std::stoll(args_[3]);
struct Command {
    Handler handler;
    size_t minArgs;
    size_t maxArgs;
    bool firstIsKey;   // args[0] is a key and must be at most kMaxKeyLength bytes
    bool restOfLine;   // the last argument is the rest of the line, spaces included
};
//                         handler                  min max  key?   rest of line?
commands_["SET"]      = {&Server::handleSet,      2, 2, true,  true};
commands_["GET"]      = {&Server::handleGet,      1, 1, true,  false};
commands_["APPEND"]   = {&Server::handleAppend,   2, 2, true,  true};
commands_["GETRANGE"] = {&Server::handleGetRange, 3, 3, true,  false};
...
// in handle():
if (args.size() < cmd.minArgs || args.size() > cmd.maxArgs)
    return "ERR wrong number of arguments for '" + toLower(name) + "'";
if (cmd.firstIsKey && args[0].size() > kMaxKeyLength) return "ERR key too long";
return (this->*cmd.handler)(args);

混杂与连体 → 通用的切词 + 显式参数

void Server::tokenize(const std::string& line) {
    ...
        if (args_.size() == 2) {
            std::string cmd = toUpper(args_[0]);
            if (cmd == "SET" || cmd == "APPEND") {
                ...
                rawValue_ = line.substr(i);   // 只有这两个命令读它
                return;
            }
        }
}
std::string Server::handleSet() {
    if (args_.size() != 2 || rawValue_.empty()) ...   // 为什么是 2?
// Splits `text` into words separated by runs of whitespace. If `maxWords` is
// not 0 and the text has more words than that, the last word returned is
// everything from the start of word number `maxWords` to the end of the
// text, whitespace included.
std::vector<std::string> splitWords(const std::string& text, size_t maxWords);

// in handle():
Args args;
if (nameAndRest.size() == 2)
    args = splitWords(nameAndRest[1], cmd.restOfLine ? cmd.maxArgs : 0);

std::string Server::handleSet(const Args& args) {
    try {
        db_.set(args[0], args[1], writeOptions_);
        return "OK";
    } ...

整数解析合成一个 parseInt64()(src/parse_int.h),INCR、GETRANGE 和 Store::incr() 共用。它值得独立,是因为它藏住的是真正容易写错的东西,而签名只有"字符串进、成功与否和数值出"。

数字:参数个数检查 10 处 → 1 处,键长检查 6 处 → 1 处,整数解析 4 处 → 1 处;server.cpp 232 → 211 行,加上新的 parse_int.* 35 行。测试一行未改。

两处回复变了

输入基线参考答案
APPEND <超过 256 字节的键> x(integer) 1(漏了检查)ERR key too long,和 SET 一致
GETRANGE k 0 1x当成 0 1 执行ERR start and end must be integers,和 INCR 一致

这两个都是重复代码"走样"留下的 bug。消除重复的直接回报,就是这类不一致以后不会再出现。

代价与取舍

命令表每行 5 个字段,Command 的注释要解释每一个,这也是要学的接口。它划算,是因为它替换了 10 份副本,而且每个字段都是命令本来就有的属性。反例就在眼前:每个处理函数里的两三行 try/catch 如果硬抽成一个带一堆参数的辅助函数,签名会比被替换的代码还难懂。那部分留给 m06,用另一种思路(在一处聚合处理异常)解决。

继续拆下去很诱人:checkArity()、validateKey()、formatInteger()……每个两三行。那样得到的是一串浅函数,读一个命令要在五六处之间跳,又回到了"连体"。参考答案只在一个地方切开:handle() 负责"这一行是不是合法的命令调用",处理函数负责"这个命令做什么"。判断拆得对不对,看的是两边能否各自读懂,不是行数。

splitWords 比原来"只切词"多了一个 maxWords。这是把专用知识移出去的代价。可以接受,因为这个参数本身是通用的:任何调用者都按同一个规则理解它,它不暗示任何具体命令。

审稿人仍会指出的问题

  • 每个处理函数里几乎相同的 try/catch,以及 DEL、GETRANGE 那些本可以不存在的错误:m06。
  • Store 的 set/incr/append 各自重复写日志和更新索引:m02。那是另一种重复,解法是更通用的接口,而不是抽辅助函数。
  • handleGet() 先 exists() 再 get(),bulk(s, true, true) 的两个 bool 看不出意思:m08。

完整讨论见 solutions/m05/DISCUSSION.md。对照自己的改动:

cd project            # 仓库里 minikv/ 和 solutions/ 所在的目录
make -C minikv clean && make -C solutions/m05 clean
diff -ru minikv solutions/m05
make -C solutions/m05 test