-
Notifications
You must be signed in to change notification settings - Fork 58
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
[WIP] relationdomain refactor ( fork from old PR ) #145
base: main
Are you sure you want to change the base?
Conversation
On the basis of retaining the original code, the implementation of the interface has been replaced, generally completing the architectural direction of the OOP refactoring.
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.
有一些小问题可能得改一下,另外需要 @BaiZe1998 来看看
applications/relationDomain/dal/repository/relation/repository.go
Outdated
Show resolved
Hide resolved
applications/relationDomain/dal/repository/relation/repository.go
Outdated
Show resolved
Hide resolved
感谢review,三个问题先改了 |
不好意思,我可能搞得不是很清楚,目前这个pr是个什么进度? 现阶段的重构大概是:
然后如果现有代码没问题,我就有空接着写了,后面的目标大概是干掉 |
} | ||
|
||
if len(ret) <= 0 { | ||
return ret, redis.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.
这里我认为可能没必要返回redis.Nil
,这个报错语意应该偏向没有从redis中获取到结果,但这里应该是获取到了结果或是一个空列表
现有代码我认为整体上没啥大问题。 关于Redis部分Redis这里的部分代码可能同质性比较高,比如粉丝数/关注数,粉丝列表/关注列表/朋友列表等。这些内容涉及Redis的代码都大差不差,所以我认为可以考虑封装一个方法,用于向Redis存储数字信息或列表信息,然后这几个Redis的代码再去调用。 关于进度不用担心,慢慢来即可 其他主分支上现在取消了这种 也可以先按你当前的进度搞,后面我帮忙把你的代码调整一下,然后你再继续提起PR |
好的明白,后面有空先看看怎么和主分支同步,然后考虑一下redis这边怎么封装,后面继续写 |
Link to an issue
开了新分支所以重新创建了pr,原来的是这个#138
对应issue:#94
What
在原来基础上主要对 redis 这边做了进一步重构
How
参考 commentDomain 抽象了多个 redis 对象
目前进度
这个重构我拖了很久了,我感觉对我来说还是很有难度的,相较于
userDomain
和commentDomain
两个感觉情况都有不一样,留给了我自己太多设计的余地了。我现在对自己的设计总是因为找不到太多依据所以不是很有自信,可能当初还是有点错误预估难度了,非常抱歉。
所以我还是想就是先发出来想麻烦帮忙看看这样重构是否合理,然后等有集中时间再考虑一下后续设计,讨论好再进行后面的重构。
Checklist
rebase
to confirm that current branch doesn't conflict with main branch.guidelines.md
which used to describe how to build, deploy and use DouTok.