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

Question

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

1. 在表中查找特定地址并返回 ID（如果已存在）

2. 如果没有，请为此特定地址创建一条新记录

3. 返回这条新创建记录的 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 != nilfunc 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。