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

Question

我写了一个应该做一件简单事情的函数：在表中查找特定地址并返回 ID（如果已存在）如果没有，请为此特定地址创建一条新记录返回这条新创建记录的 ID作为 RDMS，我在这里使用 mysql。我将所有内容放在事务中，以避免在调用此函数的并发 go-routines 中出现竞争条件。然而，大量的不断检查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(...)被分配id，因此检查将正确反映返回值。这可能有点棘手，因此您可能更喜欢更明确地编写它，尤其是现在该函数要短得多。errdeferif 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 是最好的 Go。