Skip to content

Conversation

@HelgeStenstrom
Copy link
Collaborator

This implementation doesn't subclass StreamPlayer, and it doesn't implement the StreamPlayerListener interface. Instead, the application has an instance of the regular StreamPlayer, and there is a separate implementation of the StreamPlayerListener interface.

One thing I dislike, is that the StreamPlayerListener depends on the StreamPlayer; it imports it, and has an instance of it, because that is how it gets the total number of bytes.

I have also added an alternative progress logging message.

I would have prefered to create the listener i main(), but it needs information that only the application has (the filename), so it has to be created in the application.

Besides that, it works as usual.

This implementation doesn't subclass StreamPlayer, and it doesn't implement the StreamPlayerListener interface. Instead, the application has an instance of the regular StreamPlayer, and there is a separate implementation of the StreamPlayerListener interface.

One thing I dislike, is that the StreamPlayerListener depends on the StreamPlayer; it imports it, and has an instance of it, because that is how it gets the total number of bytes.

I have also added an alternative progress logging message.

I would have prefered to create the listener i main(), but it needs information that only the application has (the filename), so it has to be created in the application.

Besides that, it works as usual.
@goxr3plus
Copy link
Owner

Go for it :)

HelgeStenstrom and others added 9 commits September 1, 2019 18:51
Generally, it's not recommended to throw Exception. See for example https://rules.sonarsource.com/java/RSPEC-112

In class StreamPlayer, new Exception is changed into new UnsupportedOperationException

As a consequence, StreamPlayerEventLauncher.call() no longer throws a checked exception. UnsupportedOperationException is an unchecked exception.
Mockito is a mocking library used to create faked instances of classes. The class doesn't need to exist yet; we can create a mock from an interface or a parent class.

Mockito can also create spies, which intercept existing methods and check if and how they are called. The demonstration unit test uses spies.

Mockito has JUnit as a dependency. To avaoid JUnit version number conflict, we back the Junit version from 5.5.1 to 5.1.1, which is what this verison of Mockito wants.
Mockito is a mocking library used to create faked instances of classes. The class doesn't need to exist yet; we can create a mock from an interface or a parent class.

Mockito can also create spies, which intercept existing methods and check if and how they are called. The demonstration unit test uses spies.

Mockito has JUnit as a dependency. To avaoid JUnit version number conflict, we back the Junit version from 5.5.1 to 5.1.1, which is what this verison of Mockito wants.
This implementation doesn't subclass StreamPlayer, and it doesn't implement the StreamPlayerListener interface. Instead, the application has an instance of the regular StreamPlayer, and there is a separate implementation of the StreamPlayerListener interface.

One thing I dislike, is that the StreamPlayerListener depends on the StreamPlayer; it imports it, and has an instance of it, because that is how it gets the total number of bytes.

I have also added an alternative progress logging message.

I would have prefered to create the listener i main(), but it needs information that only the application has (the filename), so it has to be created in the application.

Besides that, it works as usual.
@HelgeStenstrom
Copy link
Collaborator Author

This pull request is a draft, because I want some comments on it. The main difference compared with the previous demo application, is that this one is not a subclass of StreamPlayer. That is an advantage in my opinion, but it's a matter of preferred style. There are no feature differences; they do exactly the same thing, so keeping both variants seems a bit meaningless.

StreamPlayer has a huge amount of public methods, and many of them are not used. I'm wondering if the unused methods should be removed, or if they should be used by the demo application, or at least by some unit tests. I don't think they should remain unused. If you want to showcase them in an application, then it might be worthwhile to have both a minimal application, and one that use all or most of the available methods in StreamPlayer.

This implementation doesn't subclass StreamPlayer, and it doesn't implement the StreamPlayerListener interface. Instead, the application has an instance of the regular StreamPlayer, and there is a separate implementation of the StreamPlayerListener interface.

One thing I dislike, is that the StreamPlayerListener depends on the StreamPlayer; it imports it, and has an instance of it, because that is how it gets the total number of bytes.

I have also added an alternative progress logging message.

I would have prefered to create the listener i main(), but it needs information that only the application has (the filename), so it has to be created in the application.

Besides that, it works as usual.
@goxr3plus goxr3plus marked this pull request as ready for review September 3, 2019 12:27
@goxr3plus
Copy link
Owner

Let me see this tomorrow at work.

@HelgeStenstrom HelgeStenstrom self-assigned this Sep 4, 2019
@HelgeStenstrom HelgeStenstrom added the improvement Refactorings and internal structure improvements label Sep 4, 2019
@goxr3plus
Copy link
Owner

Yes i have to make a documentation on how to use all the methods . Actually all of them have a purpose ,XR3Player is using most of them .

We need a documentation somewhere :)

@goxr3plus goxr3plus merged commit ea4b7d9 into goxr3plus:master Sep 4, 2019
@HelgeStenstrom HelgeStenstrom deleted the anotherMain branch September 5, 2019 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Refactorings and internal structure improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants