goxr3plus / java-stream-player

🌌Java Advanced Audio Controller Library (WAV, AU, AIFF, MP3, OGG VORBIS, FLAC, MONKEY's AUDIO and SPEEX audio formats )
GNU General Public License v3.0
147 stars 33 forks source link

Remove dead code implementing awaitTermination() #34

Open HelgeStenstrom opened 5 years ago

HelgeStenstrom commented 5 years ago

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.

HelgeStenstrom commented 5 years ago

If merged, this closes issue #2.

goxr3plus commented 5 years ago

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.

goxr3plus commented 5 years ago

I specifically remember writing and using this method in XR3Player because i had issues

goxr3plus commented 5 years ago

But you might be right after all, let me think.

goxr3plus commented 5 years ago

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 ♥

HelgeStenstrom commented 5 years ago

Rebased, to remove merge conflict.

HelgeStenstrom commented 5 years ago

See for yourself.

Current master branch, line 657: https://github.com/goxr3plus/java-stream-player/blob/e50245f11518a91309ccefbcdc2a7d977a21b529/src/main/java/com/goxr3plus/streamplayer/stream/StreamPlayer.java#L657

But there is no place where future is assigned a value. The default value set on line 145 is null. https://github.com/goxr3plus/java-stream-player/blob/e50245f11518a91309ccefbcdc2a7d977a21b529/src/main/java/com/goxr3plus/streamplayer/stream/StreamPlayer.java#L145

So the code in the if-clause will not be executed, and there is nothing else in awaitTermination(). Since awaitTermination() does nothing, the call can be removed, as well as the method itself.

HelgeStenstrom commented 5 years ago

I'm not the first one to see this. https://github.com/goxr3plus/java-stream-player/issues/2#issuecomment-471306539

goxr3plus commented 5 years ago

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 .

HelgeStenstrom commented 5 years ago

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.