Skip to content

Conversation

ookura-mf
Copy link

No description provided.

Copy link
Owner

@mf-sakura mf-sakura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ookura-mf
概ね良さそうに思いました:+1:
エラーハンドリング周りで1点だけコメントしました。

Comment on lines +73 to +74
err := userController.Update(*id, *firstName, *lastName)
if err != nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goだと以下の様にする事が多いです。
1行になってシンプルになったり、変数のスコープがif文に狭まるのでメモリ効率が良いです。

Suggested change
err := userController.Update(*id, *firstName, *lastName)
if err != nil {
if err := userController.Update(*id, *firstName, *lastName); err != nil {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

なるほど 👀
rubyだとifで代入するのは可読性が悪いので設定によってはlinterに怒られたりしますが、ここではerrに代入するだけなのでこの方が良さそうですね

Comment on lines +42 to +43
_, err := tx.Exec("UPDATE users SET first_name = ?, last_name = ? WHERE id = ?", u.FirstName, u.LastName, u.ID)
if err != nil {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これも一つ目の戻り値を使わないのであれば、以下の方が良いと思います。

Suggested change
_, err := tx.Exec("UPDATE users SET first_name = ?, last_name = ? WHERE id = ?", u.FirstName, u.LastName, u.ID)
if err != nil {
if _, err := tx.Exec("UPDATE users SET first_name = ?, last_name = ? WHERE id = ?", u.FirstName, u.LastName, u.ID); err !=nil{}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants