Remove dead code implementing awaitTermination() #34
Conversation
The logarithmic gain getter gets the gain directly from the SourceDataLine. We cannot know for sure that it will be logarithmic, but it seems to behave that way.
StreamPlayer is no longer printing to System.out. Instead messages are logged. To be able to log instead of print in StreamPlayerEventLauncher, its constructor signature has been changed, so that the source is a StreamPlayer instead of an Object. The StreamPlayerEventLauncher uses the logger of the source.
As it was, the field future was always null, and therefore the method awaitTermination() didn't do anything. So I removed awaitTermination() and the future field. They were not used, therefore they were not needed. If something else is needed (if there is still a need, even if it wasn't fulfilled by the removed method), it's best to write a JUnit testcase that shows how the need is fulfilled by new code.
|
If merged, this closes issue #2. |
Remove printing to System.out
|
Wait wait, are you sure about this one, when i wrote Future i used it because it was useful for a multithreading issue. Can you apply the change to your repo and check if the library works correctly with XR3Player, we might have issues in production after this. |
|
I specifically remember writing and using this method in XR3Player because i had issues |
|
But you might be right after all, let me think. |
|
O my gosh i am writing REACT js everyday and i have 1 million thinks in my mind, thanks for detecting important issues, i appreciate you very much ♥ |
As it was, the field future was always null, and therefore the method awaitTermination() didn't do anything. So I removed awaitTermination() and the future field. They were not used, therefore they were not needed. If something else is needed (if there is still a need, even if it wasn't fulfilled by the removed method), it's best to write a JUnit testcase that shows how the need is fulfilled by new code.
…elgeStenstrom/java-stream-player into removeDeadCodeAwaitTermination
|
Rebased, to remove merge conflict. |
|
See for yourself. Current master branch, line 657: But there is no place where future is assigned a value. The default value set on line 145 is null. So the code in the if-clause will not be executed, and there is nothing else in awaitTermination(). |
|
I'm not the first one to see this. |
Unit tests for gain, new getter for logarithmic gain level.
|
Yes you are right , i had a careful look now , it seems i added it because i wanted to implement something . I need to think , await termination has a role to play , i will remember . |
|
I suggest that you remove the dead code from the master branch now (merge the pull request), and then think about what it was that you wanted to accomplish, do some unit test for it, and then write the code to make them pass. |
Minor comment fixes
Alphabetically sorted methods
This reverts commit 409bf9b.
Also extracted flushAndStopOutlet, but it's not tested yet, so it will be moved when it's tested.
Test that paueses, to exercise flushAndStopOutlet()
…treamPlayer Extract class from stream player
As it was, the field future was always null, and therefore the method awaitTermination() didn't do anything. So I removed awaitTermination() and the future field. They were not used, therefore they were not needed. If something else is needed (if there is still a need, even if it wasn't fulfilled by the removed method), it's best to write a JUnit testcase that shows how the need is fulfilled by new code.
…elgeStenstrom/java-stream-player into removeDeadCodeAwaitTermination
As it was, the field future was always null, and therefore the method awaitTermination() didn't do anything. So I removed awaitTermination() and the future field.
They were not used, therefore they were not needed.
If something else is needed (if there is still a need, even if it wasn't fulfilled by the removed method), it's best to write a JUnit testcase that shows how the need is fulfilled by new code.