如何在没有所有这些级联错误圣诞树的情况下编写干净的代码

Question

我写了一个函数，它应该做一件简单的事情：

1. 在表中查找特定地址并返回ID，如果已存在
2. 如果没有，请为此特定地址创建一个新记录
3. 返回这个新创建的记录的ID

作为RDMS，我在这里使用mysql。我把所有东西都放在一个事务中，以避免在调用该函数的并发go例程中出现竞争条件。

然而，对err的大量不断检查使代码变得丑陋，并且很难获得完整的测试覆盖率。

在提高代码质量方面，我有什么可以改进的吗？

func getAddressId(db *sql.DB, address string) (int64, error) {
tx, err := db.Begin()
if err != nil {
tx.Rollback()
return 0, err
}
stmt, err := tx.Prepare("SELECT id FROM address WHERE `address`=?")
if err != nil {
tx.Rollback()
return 0, err
}
defer stmt.Close()
var result sql.NullInt64
err = stmt.QueryRow(address).Scan(&result)
if err != nil && err != sql.ErrNoRows {
tx.Rollback()
return 0, err
}
if result.Valid {
tx.Commit()
return result.Int64, nil
}
stmt, err = tx.Prepare("INSERT INTO address (address) VALUES (?)")
if err != nil {
tx.Rollback()
return 0, err
}
var res sql.Result = nil
res, err = stmt.Exec(address)
if err != nil {
tx.Rollback()
return 0, err
}
tx.Commit()
var id int64 = 0
id, err = res.LastInsertId()
return id, err
}

Answer 1

首先，也是最重要的一点，上面的代码没有什么错误。我会调整一些部分(并将在下面进行调整(，但总的来说，它非常清晰、直接，(几乎(很难出错。这一点也不难看。

其次，请参阅错误处理和Go，了解有关错误处理Go的想法，尽管我不会在这里使用这些技术，因为它们不是必需的。

现在有一件事有点糟糕，那就是很容易忘记在正确的位置调用tx.Rollback()或tx.Commit()。在我看来，这是合理的修复(但它实际上更多的是风格而非实质(。以下内容未经测试。

// Name your return values so that we can use bare returns.
func getAddressId(db *sql.DB, address string) (id int64, err error) {
tx, err := db.Begin()
if err != nil {
return // This is a bare return. No need to write "0, err" everywhere.
}
// From this point on, if we exit with an error, then rollback, otherwise commit.
defer func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
}()
stmt, err := tx.Prepare("SELECT id FROM address WHERE `address`=?")
if err != nil {
return
}
defer stmt.Close()  // I'm not sure this is correct, because you reuse stmt
// This is purely style, but you can tighten up `err = ...; if err` logic like this:
var result sql.NullInt64
if err = stmt.QueryRow(address).Scan(&result); err != nil && err != sql.ErrNoRows {
return
}
if result.Valid {
id = result.Int64
return
}
if stmt, err = tx.Prepare("INSERT INTO address (address) VALUES (?)"); err != nil {
return
}
res, err := stmt.Exec(address)
if err != nil {
return
}
id = res.LastInsertId()
}

也就是说，我认为这个函数做得太多了，如果你把它分解，它会变得更容易理解。例如(同样，未经测试(：

func getExistingAddressId(tx *sql.Tx, address string) (id int64, err error) {
stmt, err := tx.Prepare("SELECT id FROM address WHERE `address`=?")
if err != nil {
return
}
// I believe you need to close both statements, and splitting it up makes that clearer
defer stmt.Close()
var result sql.NullInt64
if err = stmt.QueryRow(address).Scan(&result); err != nil && err != sql.ErrNoRows {
return
}
// This is probably over-complicated. If !Valid, then .Int64 is 0.
if result.Valid {
return result.Int64, nil
}
return 0, nil
}
func insertNewAddress(tx *sql.Tx, address string) (id int64, err error) {
stmt, err := tx.Prepare("INSERT INTO address (address) VALUES (?)")
if err != nil {
return
}
defer stmt.Close()
res, err := stmt.Exec(address)
if err != nil {
return
}
return res.LastInsertId()
}
func getAddressId(db *sql.DB, address string) (id int64, err error) {
tx, err := db.Begin()
if err != nil {
return
}
defer func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
}()
if id, err = getExistingAddressId(tx, address); err != nil || id != 0 {
return
}
return insertNewAddress(tx, address)
}

使用这样的命名返回值是一个风格问题，当然不能这样做，这也同样清楚。但要点(a(defer是一种强大的方法，可以避免重复必须始终运行的逻辑；(b(如果一个函数在错误处理方面一团糟，它可能做得太多了。

顺便说一句，我强烈怀疑你可以取消这里的Prepare调用，这将大大简化事情。您只能使用语句一次。如果您缓存了这些语句并重用它们，那么准备它们是有意义的。如果你这样做，那么代码简化为：

func getExistingAddressId(tx *sql.Tx, address string) (int64, error) {
var result sql.NullInt64
if err := tx.QueryRow("SELECT id FROM address WHERE `address`=?", address).
Scan(&result); err != nil && err != sql.ErrNoRows {
return 0, err
}
return result.Int64, nil
}
func insertNewAddress(tx *sql.Tx, address string) (int64, error) {
res, err := tx.Exec("INSERT INTO address (address) VALUES (?)", address)
if err != nil {
return 0, err
}
return res.LastInsertId()
}
func getAddressId(db *sql.DB, address string) (id int64, err error) {
tx, err := db.Begin()
if err != nil {
return 0, err
}
defer func() {
if err != nil {
tx.Rollback()
} else {
tx.Commit()
}
}()
if id, err = getExistingAddressId(tx, address); err != nil || id != 0 {
return
}
return insertNewAddress(tx, address)
}

这不是试图简化Go语法，而是简化了操作，这是一个副作用，使语法更简单。

如果您不太熟悉命名的返回值，可能会忽略一个小细节。在return insertNewAddress(...)中，函数调用的返回值在defer运行之前被分配给id和err，因此if err != nil检查将正确反映返回值。这可能有点棘手，所以你可能更喜欢更明确地写这一切，尤其是现在函数要短得多。

func getAddressId(db *sql.DB, address string) (int64, error) {
tx, err := db.Begin()
if err != nil {
return 0, err
}
var id Int64
id, err = getExistingAddressId(tx, address)
if err == nil && id == 0 {
id, err = insertNewAddress(tx, address)
}
if err != nil {
tx.Rollback()
return 0, err
}
tx.Commit()
return id, nil
}

现在，代码非常简单，没有任何技巧，这是IMO的最佳选择。