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

chore: 转移 cache 初始化到 Wechat 结构体方法下 #668

Merged
merged 1 commit into from Apr 13, 2023
Merged

Conversation

jony4
Copy link
Contributor

@jony4 jony4 commented Apr 12, 2023

chore: 移除 panic,转移 cache 初始化到 Wechat 结构体方法下;在使用时可以只设置一次 cache 同时避免 panic 出现

@houseme
Copy link
Collaborator

houseme commented Apr 12, 2023

是不是可以统一前置处理

@jony4
Copy link
Contributor Author

jony4 commented Apr 13, 2023

是不是可以统一前置处理

对,目前是将在 openplatform 下的设置提前到了包入口 wechat.go 里边

@houseme houseme requested a review from silenceper April 13, 2023 00:42
@houseme
Copy link
Collaborator

houseme commented Apr 13, 2023

是不是可以统一前置处理

对,目前是将在 openplatform 下的设置提前到了包入口 wechat.go 里边

这个wc.cache 不一定有值,这样会导致需要使用cache时还是空的,需要这样处理,建议文档也一起修改

@silenceper
Copy link
Owner

没明白为什么这么调整; cache 如果是必须的为什么不panic?

@jony4
Copy link
Contributor Author

jony4 commented Apr 13, 2023

没明白为什么这么调整; cache 如果是必须的为什么不panic?

func (s *Service) NewWechat() (*wechat.Wechat, error) {
	wc := wechat.NewWechat()
	wc.SetCache(s.Store.Redis)

	s.officialAccount = wc.GetOfficialAccount(s.Config.Wechat.OfficialAccount)
	s.openPlatform = wc.GetOpenPlatform(s.Config.Wechat.OpenPlatform)

	return wc, nil
}

在实际使用时候 setCache 动作只需要执行一次就可以了。

@jony4
Copy link
Contributor Author

jony4 commented Apr 13, 2023

这个地方的核心问题是在这行代码中没有 return error 让上层感知到,于是用了 panic 处理。

func (wc *Wechat) GetOpenPlatform(cfg *openConfig.Config) *openplatform.OpenPlatform

@jony4
Copy link
Contributor Author

jony4 commented Apr 13, 2023

这个地方的核心问题是在这行代码中没有 return error 让上层感知到,于是用了 panic 处理。

func (wc *Wechat) GetOpenPlatform(cfg *openConfig.Config) *openplatform.OpenPlatform

从最佳实践的角度看,函数返回值必须带一个 error 交给上层去处理,而不是在函数体内进行 panic 操作

这样子一路 return 到最上层业务调用时就可以根据是否有 error 来做相应操作了。

@jony4
Copy link
Contributor Author

jony4 commented Apr 13, 2023

没明白为什么这么调整; cache 如果是必须的为什么不panic?

func (s *Service) NewWechat() (*wechat.Wechat, error) {
	wc := wechat.NewWechat()
	wc.SetCache(s.Store.Redis)

	s.officialAccount = wc.GetOfficialAccount(s.Config.Wechat.OfficialAccount)
	s.openPlatform = wc.GetOpenPlatform(s.Config.Wechat.OpenPlatform)

	return wc, nil
}

在实际使用时候 setCache 动作只需要执行一次就可以了。

另外 cache 作为外部资源在使用时候,只需要初始化一次即可,所以在使用 cache 的时候复用wc.Cache更佳

@silenceper silenceper merged commit d6371c7 into silenceper:v2 Apr 13, 2023
6 checks passed
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.

None yet

3 participants