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

Refactored Event Queue #806

Merged
merged 3 commits into from
Oct 21, 2018
Merged

Refactored Event Queue #806

merged 3 commits into from
Oct 21, 2018

Conversation

npathai
Copy link
Contributor

@npathai npathai commented Oct 16, 2018

  1. Test cases were not stopping AudioService before ending test case, but test comments suggested differently.
// test that service is finished
 assertFalse(!audio.isServiceRunning()); // which is equivalent to service is running
  1. Changed Audio to be a good singleton, previously because of Audio being bad singleton, test cases which were using static methods could have caused intermittent failures. After changes now each test case has dedicated instance of Audio service which provides isolation between test cases.

  2. Made stop() method blocking till service stops

  3. Made some other refactorings as well

Todo

  • Update class diagram once PR is approved.

…2) Changed Audio to be a good singleton, previously because of Audio being bad singleton, test cases which were using static methods could have caused intermittent failures. 3) Made some other refactorings as well
@IAmPramod
Copy link

  1. Changed Audio to be a good singleton, previously because of Audio being bad singleton, test cases which were using static methods could have caused intermittent failures. After changes now each test case has dedicated instance of Audio service which provides isolation between test cases.

Why we are calling it as a bad singleton. It was thread-safe previously.

@npathai
Copy link
Contributor Author

npathai commented Oct 16, 2018

  1. Changed Audio to be a good singleton, previously because of Audio being bad singleton, test cases which were using static methods could have caused intermittent failures. After changes now each test case has dedicated instance of Audio service which provides isolation between test cases.

Why we are calling it as a bad singleton. It was thread-safe previously.

@IAmPramod Bad singleton because it polluted global state. It created unseen dependency between test cases and behavior would vary on how threads get interleaved. Hence the bad.

@iluwatar
Copy link
Owner

Looks good to me

@npathai
Copy link
Contributor Author

npathai commented Oct 21, 2018

@iluwatar Updated the class diagram. Ready to merge!

@iluwatar iluwatar merged commit 25ed7c0 into master Oct 21, 2018
@iluwatar iluwatar deleted the EventQueueRefactoring branch December 30, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants