Codeigniter模型优化

Question

我有两张桌子：

表A

aid name age sex email password

表B

bid idno aid

步骤应该是

• 输入、验证email和password
• 如果电子邮件和密码是正确的，请在tableB中插入随机数作为idno
• 从两个表中选择name、age、sex、email、idno，并返回一个数组作为结果。
• 如果email和password错了，什么也不做。

我用Codeigniter模型编写了以下代码

function login($data) {
    $email = $data["email"];
    $password = $data["password"];
    $sql = "select aid from tableA where email='".$email."' and password='".$password."'";
    $query = $this->db->query($sql);
    if ($query->num_rows() > 0) {
        $ret = $query->row();
        $idno = random();
        $aid = $ret->aid;
        $sql = "insert into tableB (idno, aid) values('".$idno."','".$aid."')";
        $query = $this->db->query($sql);
        if ($query) {
            $sql = "select '".$idno."' as idno, name, age, sex, email from tableA where aid='".$aid."'";
            $query = $this->db->query($sql);
            if ($query->num_rows() > 0) {
                return $query->result_array();
            }
            return $query->num_rows();
         } else {
            return false;
         }
         return false;
    }
    return $query->num_rows();
}

代码工作，但我认为代码是如此丑陋和没有效率。如何优化代码？

Answer 1

我看到的最大的事情是通过将变量与SQL连接在一起启用了大量SQL注入漏洞。使用准备好的陈述代替。

您可以通过从函数返回的早期减少代码中的嵌套。

$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

$sql = "insert into tableB (idno, aid) values('".$idno."','".$aid."')";
$query = $this->db->query($sql);
...

if (!isset($query))
    return false;

$sql = "select '".$idno."' as idno, name, age, sex, email from tableA where aid='".$aid."'";
$query = $this->db->query($sql);

if ($query->num_rows() == 0)
    return false;

return $query->result_array();

将电子邮件和密码值作为显式参数传递，而不是数组。您只需要从这个数组中得到两个值。对于一个方法来说，这不是太多的参数：

public function login($email, $password) {
    ...
}

一些代码样式的改进：

• 将空行放在if语句的上方
• 将空行放在return语句的上方

对于图形设计师来说，白空间不仅仅是一个有用的设计元素。对于代码作者来说，它是一种将相关语句组合在一起的方法，或者将一条语句称为比其他语句更重要的东西。return语句在了解该方法的作用方面非常重要。此外，代码元素上面和下面的空白行在代码的外观上给出了自然的“中断”，从而引导您的眼睛和头脑意识到它们是相关的文本段落。