Во-первых, и это самое главное, в приведенном выше коде очень мало ошибок. Есть несколько моментов, которые я бы откорректировал (и сделаю это ниже), но в целом он очень четкий, прямой и (почти) трудно ошибиться. В этом нет ничего уродливого.
Во-вторых, см. Обработка ошибок и 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)
}
Использование именованных возвращаемых значений, как это, является вопросом стиля, и вы, конечно, не могли бы сделать это таким образом, и это было бы так же ясно. Но пункт (а) defer
— это мощный способ избежать дублирования логики, которая должна выполняться всегда, и (б) если функция становится беспорядочной при обработке ошибок, возможно, она делает слишком много.
В качестве примечания, я сильно подозреваю, что вы могли бы избавиться от вызовов 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
и err
до запуска defer
, поэтому проверка 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
}
И теперь код очень простой, без уловок, что, по моему мнению, лучше всего подходит для Go.
person
Rob Napier
schedule
29.02.2020
defer
. Для ошибок: так они обрабатываются в Go. - person mkopriva   schedule 29.02.2020