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任务与约束
- 消除重复:参数个数检查、键长检查、整数解析,各只保留一份。
- 让切词函数回到通用:它不应该知道任何命令名。"哪个命令的最后一个参数可以带空格"这件事,放到描述命令的地方去。
- 拆开连体方法:处理函数需要的输入通过参数传进来,不再通过成员变量和切词函数暗中交接。
- 如果修重复时顺手修掉了副本之间的不一致,导致某些输入的回复变了,把变化列出来,说明为什么新行为是对的。
约束:
make test全绿,不许修改tests/。已有的错误回复文字保持不变。- 不要为了消灭重复把代码拆成一堆两三行的小函数。拆出来的每个函数都应该能单独读懂、签名简单。
- 只做本任务的事:每个处理函数里的
try/catch看起来也是重复,但怎么处理异常是 m06 的题目,这次先留着。
自查清单
grep -c "wrong number" src/server.cpp的结果是 1。- 切词函数里搜不到任何命令名。
Server不再有"由一个方法写、由另一个方法读"的临时成员变量。- 随便挑一个处理函数,不看其他代码就能说清楚它的参数是什么、已经被检查过什么。
- 加一个新命令(比如值里能带空格的
SETNX key value)只需要改一处登记加一个处理函数。在纸上走一遍试试。 make test全绿,编译零警告。
提示
提示 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