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

refactor: simplify code in AddMissingPort #33

Merged
merged 1 commit into from
Jun 6, 2022

Conversation

lgbgbl
Copy link
Contributor

@lgbgbl lgbgbl commented Jun 2, 2022

What type of PR is this?

refactor

What this PR does / why we need it (English/Chinese):

en:

  1. When the length of substr is 1, we can call strings.IndexByte directly rather than calling len() for substr and then calling strings.IndexByte in strings.Index.
  2. We can assign variable as string type instead of int type so that we are able to leave out the work about converting integer to string.
  3. net.JoinHostPort() will judge whether : is in addr again, and will concat host and port if : is not in addr. However, when we call net.JoinHostPort in AddingMissingPort, : should not in addr. So we can also concat host and port directly instead of calling net.JoinHostPort.

zh:

  1. 当子串长度确定为1时,可以直接调用 strings.IndexByte 函数而不是像 strings.Index 一样先调用 len() 判断子串长度后再调用 strings.IndexByte 函数.
  2. 为省去整型数字转字符串的工作,可以将相关变量直接定义成string类型而不是int类型。
  3. net包下的 JoinHostPort 函数会再次判断 : 是否在 addr 中,如果不在则将 host 与 port 相关字符串连接起来。然而在 AddingMissingPort 函数中调用 net.JoinHostPort 时,:应不在 addr 中。所以在此可以不调用 net.JoinHostPort,而是直接连接 host 和 port 信息。

Which issue(s) this PR fixes:

None

welkeyever
welkeyever previously approved these changes Jun 2, 2022
Copy link
Member

@welkeyever welkeyever left a comment

Choose a reason for hiding this comment

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

lgtm

Duslia
Duslia previously approved these changes Jun 3, 2022
@Duslia Duslia force-pushed the optimize-AddMissingPort branch from aec7fa9 to b441a8b Compare June 4, 2022 01:33
@welkeyever welkeyever dismissed stale reviews from Duslia and themself via b441a8b June 6, 2022 03:45
@welkeyever welkeyever force-pushed the optimize-AddMissingPort branch from b441a8b to f88960b Compare June 6, 2022 03:46
Copy link
Member

@welkeyever welkeyever left a comment

Choose a reason for hiding this comment

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

lgtm

@Duslia Duslia merged commit 8278211 into cloudwego:develop Jun 6, 2022
@lgbgbl lgbgbl deleted the optimize-AddMissingPort branch June 7, 2022 16:38
FGYFFFF pushed a commit to FGYFFFF/hertz that referenced this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants