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

Improve the performance by reducing duplicated reflect usage on the s… #3413

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

wln32
Copy link
Member

@wln32 wln32 commented Mar 21, 2024

  1. 请求开始的时候,直接反射验证数据是否合法,并返回reflect.Value,后续所有操作可以不用重复反射对象
  2. 修正了测试 ghttp_z_unit_feature_request_struct_test.go:510

@gqcn
Copy link
Member

gqcn commented Mar 21, 2024

@wln732 The ci fails.

@gqcn
Copy link
Member

gqcn commented Mar 25, 2024

涉及较核心逻辑代码调整,需要花点时间review

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Involving core logic code adjustments, it will take some time to review.

@wln32
Copy link
Member Author

wln32 commented Mar 25, 2024

涉及较核心逻辑代码调整,需要花点时间review
========
其实设置default 和in tag 那里可以直接用反射赋值了,不需要再去判断data里有没有同名的,这样实现也很直观,如果data中设置了值,那后面再走gocnv.Struct这里赋值就好了

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Involving core logic code adjustments, it will take some time to review.
========
In fact, when setting default and in tag, you can directly use reflection to assign values. There is no need to judge whether there is a value with the same name in the data. This implementation is also very intuitive. If the value is set in the data, then go to gocnv.Struct to assign the value later. Got it

@wln32 wln32 marked this pull request as draft April 23, 2024 01:09
@gqcn gqcn marked this pull request as ready for review April 24, 2024 13:05
for k, v := range data {
if v == "" {
delete(data, k)
}
Copy link
Member

Choose a reason for hiding this comment

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

空字符串也是值,这里恐怕不能直接删掉哦?!

Copy link
Member Author

Choose a reason for hiding this comment

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

这里当时考虑的是,前端传的空值没啥意义,所以就把他删掉了

@gqcn
Copy link
Member

gqcn commented Apr 24, 2024

@wln32 这块代码看起来有点花时间,能否描述下你的改动思路?

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@wln32 This code looks a bit time-consuming. Can you describe your idea of ​​changing it?

@wln32 wln32 added the wip label Apr 24, 2024
@wln32
Copy link
Member Author

wln32 commented Apr 24, 2024

@wln32 这块代码看起来有点花时间,能否描述下你的改动思路?

@gqcn
先放放吧,后面在更新,
其实这块的代码,可以先做请求校验,然后在用gconv做赋值,还有defaut和header那里也可以再优化优化

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@wln32 This code looks a bit time-consuming. Can you describe your ideas for changes?

I'll leave it for now, I'll update it later.
In fact, for this piece of code, you can first do request verification, and then use gconv to do the assignment. You can also optimize defaut and header.

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

Successfully merging this pull request may close these issues.

None yet

4 participants