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

Erase the chance of concurrent read/write. #82

Merged
merged 4 commits into from
Oct 29, 2021

Conversation

chinghongfang
Copy link
Collaborator

Solve the concurrent read/write in unit test.
Details describe in #76.

Verified

This commit was signed with the committer’s verified signature.
rezkiy37 Michael (Mykhailo) Kravchenko
@@ -59,6 +59,7 @@ void testBytes() throws InterruptedException {
getter.start();
countDownLatch.countDown();
adder.join();
getter.join();
Copy link
Contributor

Choose a reason for hiding this comment

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

這個測試可以用ThreadPool改寫嗎?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

使用ThreadPool的話會讓這個測試產生"關聯",也就是說ThreadPool的改動會影響到MetricsTest,想請問這樣是可以接受的嘛?還是用改使用ExecutorService(是java的函式庫)?

Copy link
Contributor

Choose a reason for hiding this comment

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

透過使用相同的APIs可以讓整個專案的程式碼風格更一致更好閱讀。

舉個例子,例如你這邊直接用thread、另一邊用Executor,這兩種寫出來的程式碼風格就會截然不同

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

好的,瞭解。

Verified

This commit was signed with the committer’s verified signature.
rezkiy37 Michael (Mykhailo) Kravchenko
countDownLatch.countDown();
adder.join();

ThreadPool threadPool =
Copy link
Contributor

Choose a reason for hiding this comment

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

please close the pool

Verified

This commit was signed with the committer’s verified signature.
rezkiy37 Michael (Mykhailo) Kravchenko
})
.build();
threadPool.waitAll();
threadPool.close();
Copy link
Contributor

Choose a reason for hiding this comment

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

please use try-with-resources

Verified

This commit was signed with the committer’s verified signature.
rezkiy37 Michael (Mykhailo) Kravchenko
@chia7712 chia7712 merged commit 4837d44 into opensource4you:main Oct 29, 2021
@chinghongfang chinghongfang deleted the fixMetricsTest branch November 1, 2021 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants