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

Advances tour stop when it's audio playback ends (AIC-555) #317

Merged
merged 4 commits into from
Nov 1, 2018

Conversation

sam33rdhakal
Copy link
Contributor

  • Advances tour stop when selected stop's audio translation playback ends.

@@ -362,7 +367,7 @@ class AudioPlayerService : DaggerService(), PlayerService {
}
}

fun setArticObject(_articObject: Playable, audio: AudioFileModel, resetPosition: Boolean = false) {
fun changeAudio(_articObject: Playable, audio: AudioFileModel, resetPosition: Boolean = false) {
Copy link

Choose a reason for hiding this comment

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

Method changeAudio has 28 lines of code (exceeds 25 allowed). Consider refactoring.

@@ -23,4 +27,57 @@ class TourProgressManager {
val selectedTour: Subject<Optional<ArticTour>> = BehaviorSubject.createDefault(Optional(null))
val proposedTour: Subject<Optional<Pair<ArticTour, ArticTour.TourStop>>> = BehaviorSubject.createDefault(Optional(null))
val leaveTourRequest: Subject<Boolean> = PublishSubject.create()

/**
Copy link

Choose a reason for hiding this comment

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

Method playBackEnded has a Cognitive Complexity of 27 (exceeds 20 allowed). Consider refactoring.

@@ -23,4 +27,57 @@ class TourProgressManager {
val selectedTour: Subject<Optional<ArticTour>> = BehaviorSubject.createDefault(Optional(null))
val proposedTour: Subject<Optional<Pair<ArticTour, ArticTour.TourStop>>> = BehaviorSubject.createDefault(Optional(null))
val leaveTourRequest: Subject<Boolean> = PublishSubject.create()

/**
Copy link

Choose a reason for hiding this comment

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

Method playBackEnded has 28 lines of code (exceeds 25 allowed). Consider refactoring.

* this method advances [selectedStop] to next tour stop.
*/
@SuppressLint("CheckResult")
fun playBackEnded(audioFileModel: AudioFileModel) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a @WorkerThread annotation to make it clear this shouldn't run on the main thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Updated.

audioFileDao
.getAudioByIdAsync(audioID)
.toObservable()
.subscribe { audioFile ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a simple onError to this subscription, on the off chance that audioID is malformed. Something that does nothing in production but uses Timber in debug, perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@@ -23,4 +30,61 @@ class TourProgressManager {
val selectedTour: Subject<Optional<ArticTour>> = BehaviorSubject.createDefault(Optional(null))
val proposedTour: Subject<Optional<Pair<ArticTour, ArticTour.TourStop>>> = BehaviorSubject.createDefault(Optional(null))
val leaveTourRequest: Subject<Boolean> = PublishSubject.create()

/**
Copy link

Choose a reason for hiding this comment

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

Method playBackEnded has a Cognitive Complexity of 30 (exceeds 20 allowed). Consider refactoring.

@@ -23,4 +30,61 @@ class TourProgressManager {
val selectedTour: Subject<Optional<ArticTour>> = BehaviorSubject.createDefault(Optional(null))
val proposedTour: Subject<Optional<Pair<ArticTour, ArticTour.TourStop>>> = BehaviorSubject.createDefault(Optional(null))
val leaveTourRequest: Subject<Boolean> = PublishSubject.create()

/**
Copy link

Choose a reason for hiding this comment

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

Method playBackEnded has 32 lines of code (exceeds 25 allowed). Consider refactoring.

@Cliabhach Cliabhach merged commit 42d62ab into dev Nov 1, 2018
@Cliabhach Cliabhach deleted the tour_advance branch November 1, 2018 14:50
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.

2 participants