编写整洁的代码：避免级联错误和圣诞树效应

目前golang学习网上已经有很多关于Golang的文章了，自己在初次阅读这些文章中，也见识到了很多学习思路；那么本文《编写整洁的代码：避免级联错误和圣诞树效应》，也希望能帮助到大家，如果阅读完后真的对你学习Golang有帮助，欢迎动动手指，评论留言并分享~

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

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
}

解决方案

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

其次，请参阅 Error Handling and 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) 如果函数变得混乱的错误处理，则它可能做得太多了。

作为旁注，我强烈怀疑您可以摆脱此处的“准备”调用，这将大大简化事情。您仅使用该声明一次。如果您缓存了这些语句并重用它们，那么准备它们就有意义了。如果您这样做，那么代码将简化为：

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 是 go 的最佳状态。

理论要掌握，实操不能落！以上关于《编写整洁的代码：避免级联错误和圣诞树效应》的详细介绍，大家都掌握了吧！如果想要继续提升自己的能力，那么就来关注golang学习网公众号吧！