New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor if logic #4683
Refactor if logic #4683
Conversation
callbacks/create.go
Outdated
if reflect.Indirect(rv).Kind() != reflect.Struct { | ||
break | ||
} | ||
insertID, err := result.LastInsertId() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some driver doesn't support LastInsertId
, maybe should not return an error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jinzhu I have adjusted it, please CR/MR again
Maybe as you said, can you provide an example? If so, I'll change it here
…------------------ Original ------------------
From: Jinzhu ***@***.***>
Date: Thu,Sep 9,2021 1:13 PM
To: go-gorm/gorm ***@***.***>
Cc: heige ***@***.***>, Author ***@***.***>
Subject: Re: [go-gorm/gorm] Refactor if logic (#4683)
@jinzhu commented on this pull request.
In callbacks/create.go:
> if db.RowsAffected != 0 && db.Statement.Schema != nil && db.Statement.Schema.PrioritizedPrimaryField != nil && db.Statement.Schema.PrioritizedPrimaryField.HasDefaultValue { -if insertID, err := result.LastInsertId(); err == nil && insertID > 0 { -switch db.Statement.ReflectValue.Kind() { -case reflect.Slice, reflect.Array: -if config.LastInsertIDReversed { -for i := db.Statement.ReflectValue.Len() - 1; i >= 0; i-- { -rv := db.Statement.ReflectValue.Index(i) -if reflect.Indirect(rv).Kind() != reflect.Struct { -break -} +insertID, err := result.LastInsertId()
some driver doesn't support LastInsertId, maybe should not return an error here.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
28cb41e
to
1dda68d
Compare
callbacks/update.go
Outdated
if db.Statement.Schema != nil && !db.Statement.Unscoped { | ||
for _, c := range db.Statement.Schema.UpdateClauses { | ||
db.Statement.AddClause(c) | ||
if db.Error == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this line uses a different code style?
i have adjust it, please CR/MR again.
原始邮件
发件人: ***@***.***>
收件人: ***@***.***>
抄送: ***@***.***>; ***@***.***>
发送时间: 2021年10月8日(周五) 13:54
主题: Re: [go-gorm/gorm] Refactor if logic (#4683)
@jinzhu commented on this pull request.
In callbacks/update.go:
@@ -51,39 +51,38 @@ func BeforeUpdate(db *gorm.DB) { } func Update(db *gorm.DB) { - if db.Error != nil { - return - } - - if db.Statement.Schema != nil && !db.Statement.Unscoped { - for _, c := range db.Statement.Schema.UpdateClauses { - db.Statement.AddClause(c) + if db.Error == nil {
Why does this line uses a different code style?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
callbacks/create.go
Outdated
return | ||
} | ||
|
||
db.RowsAffected, err = result.RowsAffected() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the error of RowsAffected
doesn't matter, should not rollback the whole transaction if happens
callbacks/update.go
Outdated
db.AddError(err) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return is not necessary?
ok,i will rebase master and revert it
发自我的iPhone
…------------------ Original ------------------
From: Jinzhu ***@***.***>
Date: Fri,Oct 15,2021 10:41 AM
To: go-gorm/gorm ***@***.***>
Cc: heige ***@***.***>, Author ***@***.***>
Subject: Re: [go-gorm/gorm] Refactor if logic (#4683)
@jinzhu commented on this pull request.
In callbacks/create.go:
> @@ -195,9 +197,14 @@ func CreateWithReturning(db *gorm.DB) { } } } else if !db.DryRun && db.Error == nil { -if result, err := db.Statement.ConnPool.ExecContext(db.Statement.Context, db.Statement.SQL.String(), db.Statement.Vars...); err == nil { -db.RowsAffected, _ = result.RowsAffected() -} else { +result, err := db.Statement.ConnPool.ExecContext(db.Statement.Context, db.Statement.SQL.String(), db.Statement.Vars...) +if err != nil { +db.AddError(err) +return +} + +db.RowsAffected, err = result.RowsAffected()
the error of RowsAffected doesn't matter, should not rollback the whole transaction if happens
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
6001a47
to
e841ee0
Compare
@jinzhu i has adjusted these code,please CR/MR,thanks! |
I think the readability of the code is more important. The unnecessary if...else can be completely removed. At the same time, the processing of err should be processed as early as possible, so that the logic can be read coherently, clearly and clearly, and it can also enhance the maintainability of the code. And expandability.