Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

✅ maiicy's task #13

Merged
merged 14 commits into from
Oct 14, 2023
Merged

✅ maiicy's task #13

merged 14 commits into from
Oct 14, 2023

Conversation

Mai-icy
Copy link
Contributor

@Mai-icy Mai-icy commented Oct 6, 2023

No description provided.

}

if isNotExist {
createTodoSQL := `CREATE TABLE todos (
Copy link
Member

@iyear iyear Oct 13, 2023

Choose a reason for hiding this comment

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

嵌入的大段SQL改为以下之一:

  • 独立的 .sql 文件 + go:embed
  • gorm AutoMigration


func InsertTodo(userID int, title string, dueDate string) error {
// dueDate 以文本格式(YYYY-MM-DD)表示
sqlStatement := `
Copy link
Member

Choose a reason for hiding this comment

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

使用ORM: gorm,来不及就算了

}
err2 := file.Close()
if err2 != nil {
return err2
Copy link
Member

@iyear iyear Oct 13, 2023

Choose a reason for hiding this comment

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

所有的error最好能返回上下文,否则这个error传到最顶层就一个错误信息根本找不到代码的问题在哪。

Copy link
Member

Choose a reason for hiding this comment

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

err2不需要新建,复用上面的err就好。

if err = file.Close(); err!=nil{}

return
}

c.JSON(http.StatusCreated, gin.H{"message": "用户注册成功"})
Copy link
Member

Choose a reason for hiding this comment

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

201确实是“成功”的状态码之一,不过大多数前端的拦截器会只做200 OK的。所以一般“成功”就直接返回200好了


user, err := db_handle.GetUserByUsername(loginData.Username)
if err != nil {
c.JSON(http.StatusOK, gin.H{"message": "用户名不存在"})
Copy link
Member

Choose a reason for hiding this comment

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

这类有问题的响应一般不考虑返回200了

maiicy/main.go Outdated

func main() {

databasePath := "data.db"
Copy link
Member

Choose a reason for hiding this comment

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

考虑使用 flag 包或更重的 https://github.com/spf13/cobra 等其他CLI包把这些参数可以在命令行传入,给个默认值就好


// 检查是否提供了 JWT
if tokenString == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Unauthorized"})
Copy link
Member

Choose a reason for hiding this comment

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

这里的error意义不大,在状态码已经体现了,直接 c.AbortWithStatus

)

var (
SecretKey = []byte("") // 用于签名和验证 JWT 的密钥
Copy link
Member

Choose a reason for hiding this comment

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

考虑环境变量或CLI flag传入

return
}

if !utils.IsValidUsername(RegisterData.Username) {
Copy link
Member

@iyear iyear Oct 13, 2023

Choose a reason for hiding this comment

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

这个校验可以用 binding 做替代:binding:"required,alphanum" 等类似的操作。

gin底层用的 https://github.com/go-playground/validator ,具体规则可以在README 和https://github.com/go-playground/validator/blob/master/validator_test.go 中找

Copy link
Member

@iyear iyear left a comment

Choose a reason for hiding this comment

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

LGTM👏提了一些细节上的建议,整体代码风格和工程能力很不错

@NX-Official NX-Official merged commit e2e7cfa into hduhelp:main Oct 14, 2023
1 check passed
@NX-Official
Copy link
Member

NX-Official commented Oct 16, 2023

你叫什么名字哇 ❤️ @Mai-icy

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants