有什么改进这个功能的建议吗？

Question

我对使用SQL查询非常陌生。任何改进这段代码的建议： （顺便说一句，我真的不关心这里的sql安全性；这是一段将在连接到本地sqlite文件的pyexe文件中的代码—所以在这里担心查询的安全性是没有意义的）。你知道吗

def InitBars(QA = "GDP1POP1_20091224_gdp", QB = "1 pork", reset = False):
    global heights, values
    D, heights, values, max, = [], {}, {}, 0.0001

    if reset: GHolder.remove()

    Q = "SELECT wbcode, Year, "+QA+" FROM DB WHERE commodity='"+QB+"' and "+QA+" IS NOT 'NULL'"  

    for i in cursor.execute(Q): 
        D.append((str(i[0]) + str(i[1]), float(i[2])))
        if float(i[2]) > max: max = float(i[2])

    for (i, n) in D: heights[i] = 5.0 / max * n; values[i] = n
    Gui["YRBox_Slider"].set(0.0)  
    Gui["YRBox_Speed"].set(0.0)

---

在遵循建议之后，我得到的是：

def InitBars(QA = "GDP1POP1_20091224_gdp", QB = "1 pork", reset = False):
    global heights, values; D, heights, values, max, = [], {}, {}, 0.0001
    if reset: GHolder.remove()

    Q = "SELECT wbcode||Year, %s FROM DB WHERE commodity='%s' and %s IS NOT 'NULL'" % (QA, QB, QA) 
    for a, b in cursor.execute(Q): 
        if float(b) > max: max = float(b)
        values[a] = float(b)

    for i in values: heights[i] = 5.0 / max * values[i]

    Gui["YRBox_Slider"].set(0.0); Gui["YRBox_Speed"].set(0.0)

Answer 1

如果这是一个一次性脚本，您完全信任所有的输入数据，只需要完成一项工作，那么就可以了。你知道吗

如果这是系统的一部分，并且表示系统中的代码类型，则存在以下几个问题：

1. 不要通过附加字符串来构造SQL查询。你说过你不关心安全问题，但这是一个大问题，而且很容易解决，那么真的——你应该一直做得很好

2. 这个函数似乎使用和操纵全局状态。同样，如果这是一个小的一次性使用脚本，那就去做吧——在只跨几个文件的系统中，这就不可能维护了。

3. 命名约定---不遵循任何大写一致性

4. 东西的名字一点用都没有。QA，D，QB，--QA和QB似乎不是同一类的东西——一个是字段，另一个是值。

5. 所有可疑的东西都是未注释的——为什么max.0001是？戈尔德到底是什么？最后那个循环会做什么？实际上，代码应该更清楚，但如果不清楚，就给维护人员一个教训。

Answer 2

您应该检查SQL注入。确保QA中没有SQL语句。如果适用的话，您可能还应该添加斜杠。你知道吗

Answer 3

• 使用比QA和QB更具描述性的变量名。

• 注释代码。

• 不要将多个语句放在同一行中

• 尽量不要使用globals。改用成员变量。

• 如果QA和QB可能来自用户输入，请不要使用它们来构建SQL查询