不久前，我从MMO逆向工程了一种校验和算法，该算法用于检查与聊天链接的项的有效性（类似于WoW）。这个想法是，如果校验和无效，那么游戏客户端在单击时将忽略该链接。否则，在游戏中的聊天室中单击物品链接将显示该物品的状态和属性。

用字符串格式的空格分隔。它在运行程序时作为参数传递给itemlink。

这是main()的十六进制字符串的样子：

该代码段中是否存在任何代码异味或可读性问题？它的任何部分都可以改进吗？

#1 楼

如果您使用的是同名的标准库类，则为以下名称提供正确的名称空间限定符：std::string，std::stringstream和std::hex。 


uint32 hexform[ITEMLINKGROUPCOUNT] = {};


ebx，edi，ecx，eax并不是很好的变量名，如果可以给它们提供更有意义的名称，那就去做。 br /> 
    uint32 ecx = (hexform[i] + 1) * hexsum,
           eax = ecx * hexform[i];


我个人认为，这更清楚：值hexform[i]^2 + hexform[i] * hexsum和ecx获得值hexform[i] * hexsum + hexsum。我认为，如果代码符合您的意图，则注释需要一对括号。

为稳健起见，应检查该解析是否起作用。 >
您也可以简单地将前两个for循环组合在一起。

评论

\ $ \ begingroup \ $
我想变量名来自汇编程序的C ++“翻译”。因此，如果您将汇编代码视为“规范”，则名称eax，ecx等将是一个不错的选择。
\ $ \ endgroup \ $
–罗迪
11年2月2日在21:13
\ $ \ begingroup \ $
re：名称空间限定符：如果该代码在头文件中，则表示同意。但在.cpp文件中，我个人更希望看到using语句。
\ $ \ endgroup \ $
–罗迪
11年2月2日在21:17
\ $ \ begingroup \ $
@Roddy：您的意思是使用声明还是使用指令？如果有足够的using声明，我会好起来的-尽管对于std名称空间而言，这似乎几乎不值得。我认为使用指令很少是一个好主意。
\ $ \ endgroup \ $
– CB Bailey
2011-2-3在11:23

#2 楼



C ++中的函数和变量名称通常是camelCase或snake_case，而大写名称是用户定义的类型。

如果ITEMLINKGROUPCOUNT是变量或常量，则应也请遵循camelCase或snake_case。全大写字母命名通常用于宏，并且不同的单词应使用下划线分隔。

uint32不是标准的C ++，也不是C（即uint32_t）。使用std::uint32_t中的<cstdint>。有关此处和此处的更多信息。

代替此求和循环：

for (int i = 0; i < ITEMLINKGROUPCOUNT; ++i)
    hexsum += hexform[i];


使用std::accumulate作为更干净且类似于C ++的替代方法：

uint32 hexsum = std::accumulate(hexform, hexform+ITEMLINKGROUPCOUNT, 0);


另外，hexsum应该在这里初始化，因为它是第一次使用的地方。始终使变量尽可能在范围内。



#3 楼

在我看来，您的代码目前显示出这样一个事实，即它是对汇编语言进行反向工程的，这超出了我的期望。 d通常编写C ++而不是基本上只是将汇编语言转换为C ++语法，而是保留大多数汇编语言风格（直到并仍然包括使用变量的寄存器名）。 ，读取输入字符串并将其从字符串转换为uint32的向量可能更像这样：

/> 
class hex_word {
    uint32 val;
public:
    friend std::istream& operator>>(std::istream &is, hex_word &h) { 
        return is >> hex >> h;
    }
    operator uint32() { return val; }
};

std::vector<uint32> hexform{std::istream_iterator<hex_word>(parseitemlink),
                            std::istream_iterator<hex_word>()};


向前跳过一点，您的最后一个循环可以使用std::accumulate：

uint32 hexsum = std::accumulate(hexform, hexform+ITEMLINKGROUPCOUNT, 0);


我不确定会导致令人震惊的新见解或代码的极大简化，但与将所有代码混在一起相比，它仍然让我更容易理解。

#4 楼

我能看到的唯一有问题的是您的缩写变量名，即（eax，ecx，ebx和edi）没有为那些不熟悉校验和的人清楚地解释了变量的存储方式。对于可读性问题，我可以看到的另一件事是，所有具有描述性的小写变量名称（即parseitemlink）都应被驼峰化，这样我才能分辨出不同的单词（即parseItemLink）。这是我唯一能想到的可读性。