Skip to content

[ISSUE #4531] fix unreachable statement, redundant code and code style #4532

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
Merged
merged 3 commits into from
Jul 4, 2022
Merged

[ISSUE #4531] fix unreachable statement, redundant code and code style #4532

merged 3 commits into from
Jul 4, 2022

Conversation

PansonPanson
Copy link
Contributor

I'm reading the sourcecode of store module and there are some cpde snippets in class org.apache.rocketmq.store.DefaultMessageStore that I think need to be fixed.

  • code snippet 1: redundant code and unreachable statement (match is always true)

Screen Shot 2022-06-29 at 21 34 38

  • code snippet 2 : replacing with enhanced for to Improve code readability

image

@PansonPanson PansonPanson changed the title [ISSUE #4513] elegant equals using apache-commons [ISSUE #4531] elegant equals using apache-commons Jun 29, 2022
@PansonPanson
Copy link
Contributor Author

linked issue : 4531

@PansonPanson PansonPanson changed the title [ISSUE #4531] elegant equals using apache-commons [ISSUE #4531] fix unreachable statement, redundant code and code style Jun 30, 2022
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 52.187% when pulling 15bbba3 on PansonPanson:develop into 3644941 on apache:develop.

@guyinyou
Copy link
Contributor

guyinyou commented Jul 1, 2022

lgtm

tsunghanjacktsai
tsunghanjacktsai approved these changes Jul 1, 2022
View reviewed changes
Copy link
Contributor

@tsunghanjacktsai tsunghanjacktsai left a comment

Choose a reason for hiding this comment

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

Pretty thoughtful change. I believe there are still a lot of similar issues in our code. Please could you help us improve them in the future? Really appreciate it.

@PansonPanson
Copy link
Contributor Author

Pretty thoughtful change. I believe there are still a lot of similar issues in our code. Please could you help us improve them in the future? Really appreciate it.

sure, I’m going to read the source code in the next few months.

@duhenglucky duhenglucky merged commit 06d954f into apache:develop Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tianliuliu tianliuliu tianliuliu approved these changes

@caigy caigy caigy approved these changes

@hzh0425 hzh0425 hzh0425 approved these changes

+2 more reviewers

@tsunghanjacktsai tsunghanjacktsai tsunghanjacktsai approved these changes

@guyinyou guyinyou guyinyou approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@PansonPanson @coveralls @guyinyou @tianliuliu @caigy @tsunghanjacktsai @hzh0425 @duhenglucky