Golang中你更喜欢以下哪段代码实现？
Golang中你更喜欢以下哪段代码实现？ 我正在研究代码质量（虽然这个帖子不是我研究的一部分），我只是想就两个实现相同功能但编码方式不同的代码片段征求一些意见。
示例 A:
func (r sqlUserRepo) store(u user) {
r.upsertUserRecord(u)
for _, key := range u.keyring {
if key.isRevoked() {
r.removeKey(key)
} else if key.isBumped() {
r.upsertKeyRecord(key)
}
}
}
func (r sqlUserRepo) upsertUserRecord(u user) {
_, err := r.executor.ExecContext(
r.ctx,
`
INSERT INTO users (id, username, password, email) VALUES ($1, $2, $3, $4)
ON CONFLICT(id)
DO UPDATE SET username = $2, password = $3, email = $4
`,
u.id,
u.username,
u.password,
u.email,
)
panicIfNotNil(err)
}
func (r sqlUserRepo) upsertKeyRecord(key accessKey) {
_, err := r.executor.ExecContext(
r.ctx,
`
INSERT INTO keys (id, owner, hash, expires_on) VALUES ($1, $2, $3, $4)
ON CONFLICT(id)
DO UPDATE SET owner = $2, hash = $3, expires_on = $4
`,
key.id,
key.owner,
key.hash,
key.expiresOn,
)
panicIfNotNil(err)
}
示例 B:
func (r sqlUserRepo) store(u user) {
_, err := r.executor.ExecContext(
r.ctx,
`
INSERT INTO users (id, username, password, email) VALUES ($1, $2, $3, $4)
ON CONFLICT(id) DO UPDATE SET username = $2, password = $3, email = $4 WHERE id = $1
`,
u.id,
u.username,
u.password,
u.email,
)
panicIfNotNil(err)
for _, key := range u.keyring {
if key.isRevoked() {
_, err := r.executor.ExecContext(r.ctx, `DELETE FROM keys WHERE keys.id = $1`, key.id)
panicIfNotNil(err)
} else if key.isBumped() {
_, err = r.executor.ExecContext(
r.ctx,
`
INSERT INTO keys (id, owner, hash, expires_on) VALUES ($1, $2, $3, $4)
ON CONFLICT DO UPDATE SET owner = $2, hash = $3, expires_on = $4
`,
key.id,
key.owner,
key.hash,
key.expiresOn,
)
panicIfNotNil(err)
}
}
}
请你阅读这两个代码片段，并在下方评论你更喜欢哪一个（觉得哪个更容易阅读和理解）以及原因？另外，如果你正在一个代码库上工作，你更愿意处理这两个片段中的哪一个，或者你觉得哪个更容易上手（换句话说，更容易修改）？
更多关于Golang中你更喜欢以下哪段代码实现？的实战教程也可以访问 https://www.itying.com/category-94-b0.html
我知道这有点跑题，但如果不使用 upsert，你的代码会不会存在一个罕见的竞态条件？UPSERT - PostgreSQL wiki
更多关于Golang中你更喜欢以下哪段代码实现？的实战系列教程也可以访问 https://www.itying.com/category-94-b0.html
我不是这方面的专家。我认为，如果你在包括SELECT在内的所有语句之前开始一个事务，并在所有语句之后提交，那么可能就没问题？
无论如何，遇到竞态条件的机会很罕见。我建议在代码中添加一条注释，以便以后研究upsert操作。
// 此处应添加代码示例，但原文未提供具体代码。
// 根据上下文，这里可能涉及数据库事务和SELECT语句。
// 例如：
// tx, err := db.Begin()
// if err != nil {
// log.Fatal(err)
// }
// defer tx.Rollback()
//
// // 执行SELECT或其他语句
// _, err = tx.Exec("SELECT ...")
// if err != nil {
// log.Fatal(err)
// }
//
// // 其他操作...
//
// err = tx.Commit()
// if err != nil {
// log.Fatal(err)
// }
说得好，sqlUserRepo 是一个名为 userRepo 的接口的实现。
interface userRepo {
store(user)
load(userQuery) (user, error)
exists(userQuery) bool
}
type userQuery struct {
id string
username string
email string
token string
}
type sqlUserRepo struct {
ctx context.Context
executor persistance.QueryExecutor
}
所有这些代码都在一个名为 identity 的小包内，该包主要处理用户身份验证。这就是为什么 store 方法没有导出的原因，因为应用程序的其他部分不会使用它，而是会使用 identity.Protect 这个用于验证用户请求的 HTTP 中间件。
事实上，示例 A 中的另外两个方法不应该被直接调用，应用程序代码不会直接与 sqlUserRepo 交互，而是会使用 userRepo 接口。因此，store 是唯一对用户公开可见的方法。
我的错，我应该在原帖中包含更多上下文。user 结构体附加了一个 keyring 属性，它包含一个特殊访问密钥的切片。这些密钥在身份验证过程中使用，并且具有有限的生命周期。每次用户使用一个访问密钥时，它都会被“续期”（过期日期被重置）。当用户请求一个新密钥时，新创建的密钥也会被续期。如果密钥被撤销（其过期日期已过或用户调用了 key.revoke()），那么存储库在存储用户时会删除它。
user 对象绝不能处于不良状态（所有访问密钥和其他属性在创建时都是有效的）。存储库抽象在 store 和 load 方法中保证了这一点，任何无效的访问密钥在保存用户到数据库或从数据库获取用户时都会被删除。此外，如果用户名、电子邮件和密码未通过用户构造函数的验证，load 方法也会引发恐慌。
我同意仅仅因为查询失败就引发恐慌可能不是最好的主意，但我认为如果应用程序检测到数据库损坏，调用恐慌是可以接受的。
要回答我更喜欢哪一个，我需要了解更多信息。这些函数是如何使用的？我通常喜欢将步骤分解成更小的函数，就像你的第一个例子那样。然而，由于所有方法都是未导出的，作为API的用户，我如何知道应该调用哪一个（或哪几个）？在两个示例中都定义了 store 函数，所以我的印象是，该函数是用户代码的主要“入口点”，但如果没有在第二个示例中看到它，我就不会知道这一点。
话虽如此，如果用户只需要调用 store 函数就完成了，我可能会倾向于示例B，尽管它是一个更长的函数。从用户的角度来看，sqlUserRepo 类型上的 store 方法可能被视为单一职责。然而，如果不是这样，并且用户代码可能需要知道何时更新插入用户记录或知道这些 key 是什么，那么它们可能需要保持为独立的函数。也许其他地方有这样的代码：
type UserRepo interface {
store(user)
}
type cachedUserRepo interface {
upsertUserRecord(user)
removeKey(accessKey)
upsertKeyRecord(accessKey)
}
// SaveUser 将用户保存到存储库。
// 如果存储库支持 accessKeys，则这些 key 会被全局缓存。
func SaveUser(r UserRepo, u user) {
cu := userFromCache(u)
tx := beginTx()
defer func() {
x := recover()
if x == nil {
tx.commit()
return
}
tx.rollback()
panic(x)
}()
switch r := r.(type) {
case cachedUserRepo:
r.upsertUserRecord(u)
for _, k := range u.keyring {
if k.revoked() {
decacheAccessKey(k)
r.removeKey(k)
} else {
cacheAccessKey(k)
}
if k.isBumped() {
r.upsertKeyRecord(k)
}
}
return
}
r.store(u)
}
另外，你使用 panic 来将错误传递出函数，这在 Go 中是非常不符合惯例的。错误应该被返回。
我更喜欢示例A的实现。原因如下：
可读性方面
示例A通过将数据库操作封装为命名方法，使主逻辑更清晰：
func (r sqlUserRepo) store(u user) {
r.upsertUserRecord(u) // 一目了然
for _, key := range u.keyring {
if key.isRevoked() {
r.removeKey(key) // 语义明确
} else if key.isBumped() {
r.upsertKeyRecord(key) // 意图清晰
}
}
}
可维护性方面
示例A的封装提供了更好的抽象层。当需要修改SQL语句时，只需在对应的方法中修改：
// 如果需要修改upsert逻辑，只需改这一处
func (r sqlUserRepo) upsertUserRecord(u user) {
_, err := r.executor.ExecContext(
r.ctx,
// SQL语句修改只影响这个方法
`INSERT INTO users ...`,
u.id,
u.username,
u.password,
u.email,
)
panicIfNotNil(err)
}
代码复用
示例A的方法可以被其他函数复用：
// 其他需要更新用户的地方可以直接调用
func (r sqlUserRepo) updateUserProfile(u user) {
r.upsertUserRecord(u)
// 其他业务逻辑
}
测试友好性
示例A更容易进行单元测试，因为可以mock具体的方法：
// 可以单独测试store方法的逻辑，而不依赖实际数据库
func TestStoreUser(t *testing.T) {
repo := &mockSqlUserRepo{
upsertUserRecordFunc: func(u user) {
// 验证参数
},
upsertKeyRecordFunc: func(key accessKey) {
// 验证逻辑
},
}
testUser := user{/* 测试数据 */}
repo.store(testUser)
// 断言方法调用
}
错误处理一致性
示例A保持了错误处理的一致性，每个数据库操作都有相同的错误处理模式：
func (r sqlUserRepo) upsertKeyRecord(key accessKey) {
_, err := r.executor.ExecContext(/* 参数 */)
panicIfNotNil(err) // 统一的错误处理
}
示例B将SQL语句直接嵌入业务逻辑中，导致：
- 主函数过于臃肿，混合了业务逻辑和数据库细节
- 修改SQL时需要搜索整个代码库
- 重复的SQL字符串难以维护
- 缺乏抽象层次，违反单一职责原则
在实际代码库工作中，示例A的结构更容易理解和修改，因为它遵循了关注点分离的原则，每个方法都有明确的职责。
