Skip to content
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

102300109_work3 #94

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

102300109_work3 #94

wants to merge 5 commits into from

Conversation

YXRRXY
Copy link
Contributor

@YXRRXY YXRRXY commented Jan 2, 2025

No description provided.

@mutezebra
Copy link
Collaborator

郑天浩 102300109

https://github.com/YXRRXY/todo-app

项目首页

优点

  • 有 README, 有一定的篇幅描述自己的项目实现
  • 有意义的 commit 信息
  • 通过Languages 统计看得出自己写了前端.

缺点

  • commit 格式不规范. 请参考 [这篇文章](https://juejin.cn/post/7243451555930898469)
  • 项目按照使用说明无法运行. 虽然我能看得出来是 mysql 不存在的问题, 但这确实是你的描述不详尽. 不过没关系, 接下来你会了解到什么是 docker, 然后解决这个问题

image-20250103104046107

项目

按照目录顺序来说

config

  • 会用 config 来管理配置信息很好, 但是不要把你的信息明文放在代码中. 你应该使用配置文件来管理
  • 常见的配置文件格式有 yaml,ini 个人常用的包是 viper

controller

  • 这个场景下, 你的方法不建议使用值接收者 (tc TodoController) 你应该使用指针 (tc *TodoController) 可以 google 一下相关信息, 并不复杂

  • 尽可能避免使用魔法变量比如 c.JSON(400, map[string]string{"error": "Invalid request"}) 我怎么知道你的 400 是什么东东, 可以开一个 constants 包来放置一些常用的常量. 比如:

    const (
    	InvalidRequestCode = 400
    )

    此外, http 包中也提供了状态码, 总之我们应该尽可能不使用魔法变量

  • 才发现是有一定的架构的, controller modle service. 挺好的

  • ??? 怎么有的地方又变成指针接收了? 你的 IDE 不会提醒你吗? 怎么还混着用

  • GetTodos 里面 的魔法变量

	page := 1
	pageSize := 10
  • line 54 直接声明一个指针变量而没有初始化, 可能会引发空指针问题. 对于一个 int 类型的变量, 不建议这样
  • line 83 同理, 我没有看你的具体逻辑,或许你可以保证对 status 赋值, 但这样的操作是有风险的. 可以试一下不对指针赋值直接引用你的程序会怎样
  • 总的来说代码还算简洁, 风格上问题不是特别大

docs

  • 写文档的习惯挺好的, 但不是所有的文档都需要写. 了解一下 apifox 下次你可以直接甩一个项目的链接过来, 而不是自己手写
  • 还是说一下, 写文档写注释都是好习惯

repository

  • 你方法的逻辑.... 先获取 query. 再调用??? 我对gorm 是否允许这样的使用感到怀疑, 我觉得你以一种错误的方式打开了 gorm

    , 但从逻辑上和结果上来看竟然还没啥错... 建议看一下 gorm 的文档吧, 我咋记得不应该这样用的 !!! (也可能是我的问题)

service

  • 看起来还好

main.go

  • 我说前面为啥一直没有 NewRepo NewService 之类的函数, 原来你全直接写在 main 里了. 建议了解一下设计模式中的 单例模式和工厂模式
  • 前面还说你用 config 来管理配置信息挺好的, 怎么main 里面还是一串明文写数据库信息, 我还以为你会用 config 的...
  • 堆杂的东西太多了, 又初始化变量, 又初始化数据库, 又初始化路由, 又要启动路由, 又要迁移表. 我们应该保证 main 的整洁, main 更多的只需要调用一些简单的函数来启动项目, 不要掺杂这么多逻辑, 我之前的一个项目示例:
func main() {
	Init()

	h := server.Default(server.WithHostPorts("0.0.0.0:8888"))
	h.Use(middleware.Cors())

	register(h)
	h.Spin()
}

func Init() {
	config.InitConfig()
  db.Init()
  cache.Init()
}
  • 包规划: 既然你已经分出来很多包了, 那你就把包该做的事情在包里做了, 比如前面的表迁移, 你就应该在 db 的包里写好再暴露出来一个函数, main 调用一下就好了.
    • 其次, 可以进一步的包划分, 比如 jwt 的颁发和校验部分可以单独抽出来到一个包里, server 要用的时候调用相关函数就好, 这是一种接口层级的屏蔽, 屏蔽了具体实现只暴露出来函数签名, 对于 service 来说他不需要知道这个函数(接口)是怎么做的, 他只需要知道这个函数(接口)能做什么就可以了
    • 同上: router 的部分拆出来
    • 数据库连接拆出来
    • 如果你了解了工厂模式和单例模式你应该看着那一大团的变量初始化也感觉不对劲了

总结

总的来说项目完成度是很高的, 基本上没有让人血压高的操作. 整体的层次划分也还可以, 虽然还有不少的改动空间

此外文档还算完整, 应该说有文档很好(๑•̀ㅂ•́)و✧

感觉比我刚开始写的好太多了, 很好 ! ! !

@SchwarzSail
Copy link
Collaborator

大致看了一下,在这个阶段完成得非常好👍👍👍,有明显的架构分层。
现阶段先需求知识面的广度,所以架构方面可以先暂时放一放(主要是要有有分层的思想),如果有参加青训营的话到时候再慢慢了解和学习架构方面的知识。
现在请你使用docker对你的项目进行部署(不要求前端),你可以要做的有:

  • Linux 的环境。
  • 安装 docker, 尽量使用命令行的形式操作 docker。
  • 为你的后端编写dockerfile,要求镜像体积尽量小。

@mutezebra
Copy link
Collaborator

大致看了一下,在这个阶段完成得非常好👍👍👍,有明显的架构分层。 现阶段先需求知识面的广度,所以架构方面可以先暂时放一放(主要是要有有分层的思想),如果有参加青训营的话到时候再慢慢了解和学习架构方面的知识。 现在请你使用docker对你的项目进行部署(不要求前端),你可以要做的有:

  • Linux 的环境。
  • 安装 docker, 尽量使用命令行的形式操作 docker。
  • 为你的后端编写dockerfile,要求镜像体积尽量小。

补充一下: 关于第一点可以参考 https://west2-online.feishu.cn/wiki/AUKFw7MVLidXmzkm80ScYDx5nid
如果你也有跟我相同的困境(装好之后会死机), 那推荐你使用 windows 下的 wsl2. 但是更推荐你装一个物理机, wsl2 只是一个替代方案, 不是首选

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

Successfully merging this pull request may close these issues.

3 participants