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.

Sign up for GitHub

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

Jump to bottom

[Improvement] Refactor ComposedClientReadHandler #575

Closed
3 tasks done
kaijchen opened this issue Feb 9, 2023 · 0 comments · Fixed by #570
Closed
3 tasks done

[Improvement] Refactor ComposedClientReadHandler #575

kaijchen opened this issue Feb 9, 2023 · 0 comments · Fixed by #570

Comments

@kaijchen
Copy link
Contributor

kaijchen commented Feb 9, 2023

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

What would you like to be improved?

In ComposedClientReadHandler, storage tiers are represented in int values, and it uses a lot of switch-case to get the corresponding handler.

This is error prone and difficult to understand.

How should we improve?

Represent storage tier as enum, and get handlers by EnumMap.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@xianjingfeng xianjingfeng linked a pull request Feb 10, 2023 that will close this issue
[#575] refactor: replace switch-case with EnumMap in ComposedClientReadHandler #570
Merged
xianjingfeng pushed a commit that referenced this issue Feb 10, 2023
@kaijchen
[#575] refactor: replace switch-case with EnumMap in ComposedClientRe…
9da9bb6 
…adHandler (#570)

### What changes were proposed in this pull request?
Add enum Tier, use EnumMap to replace switch-case in ComposedClientReadHandler.

### Why are the changes needed?
Make the code more concise.

### Does this PR introduce any user-facing change?
No.

### How was this patch tested?
Existing CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@kaijchen