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

Rewrite PlaybackHelper to use the SDK #3586

Merged
merged 3 commits into from
May 19, 2024

Conversation

nielsvanvelzen
Copy link
Member

@nielsvanvelzen nielsvanvelzen commented May 18, 2024

Changes

  • Add SdkPlaybackHelper implementation which is a rewrite in Kotlin using the SDK. It behaves the same as the old implementation
  • Remove old PlaybackHelper implementation
  • Require context in PlaybackHelper.getItemsToPlay

Issues

@nielsvanvelzen nielsvanvelzen added sdk-migration To fix this we need to migrate some code to the new SDK refactor Improvements to code realiability, readability and quality labels May 18, 2024
@nielsvanvelzen nielsvanvelzen added this to the v0.17.0 milestone May 18, 2024
Comment on lines +36 to +44
class SdkPlaybackHelper(
private val api: ApiClient,
private val mediaManager: MediaManager,
private val userPreferences: UserPreferences,
private val videoQueueManager: VideoQueueManager,
private val navigationRepository: NavigationRepository,
private val playbackLauncher: PlaybackLauncher,
private val playbackControllerContainer: PlaybackControllerContainer,
) : PlaybackHelper {

Check warning

Code scanning / detekt

The more parameters a function has the more complex it is. Long parameter lists are often used to control complex algorithms and violate the Single Responsibility Principle. Prefer functions with short parameter lists. Warning

The constructor(api: ApiClient, mediaManager: MediaManager, userPreferences: UserPreferences, videoQueueManager: VideoQueueManager, navigationRepository: NavigationRepository, playbackLauncher: PlaybackLauncher, playbackControllerContainer: PlaybackControllerContainer) has too many parameters. The current threshold is set to 7.
}
}

private suspend fun getItems(

Check warning

Code scanning / detekt

One method should have one responsibility. Long methods tend to handle many things at once. Prefer smaller methods to make them easier to understand. Warning

The function getItems is too long (138). The maximum length is 60.
}
}

private suspend fun getItems(

Check warning

Code scanning / detekt

Prefer splitting up complex methods into smaller, easier to test methods. Warning

The function getItems appears to be too complex based on Cyclomatic Complexity (complexity: 17). Defined complexity threshold for methods is set to '15'

val items = getItems(item, allowIntros, shuffle)

if (item.type == BaseItemKind.MUSIC_ALBUM || item.type == BaseItemKind.MUSIC_ARTIST || (item.type == BaseItemKind.PLAYLIST && item.mediaType == MediaType.AUDIO)) {

Check warning

Code scanning / detekt

Complex conditions should be simplified and extracted into well-named methods if necessary. Warning

This condition is too complex (4). Defined complexity threshold for conditions is set to '4'
}
}

else -> if (allowIntros && !playbackLauncher.useExternalPlayer(mainItem.type) && userPreferences[UserPreferences.cinemaModeEnabled]) {

Check warning

Code scanning / detekt

Line detected, which is longer than the defined maximum line length in the code style. Warning

Line detected, which is longer than the defined maximum line length in the code style.

val items = getItems(item, allowIntros, shuffle)

if (item.type == BaseItemKind.MUSIC_ALBUM || item.type == BaseItemKind.MUSIC_ARTIST || (item.type == BaseItemKind.PLAYLIST && item.mediaType == MediaType.AUDIO)) {

Check warning

Code scanning / detekt

Line detected, which is longer than the defined maximum line length in the code style. Warning

Line detected, which is longer than the defined maximum line length in the code style.
@nielsvanvelzen nielsvanvelzen merged commit a953860 into jellyfin:master May 19, 2024
6 checks passed
@nielsvanvelzen nielsvanvelzen deleted the sdk-playback-helper branch May 19, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improvements to code realiability, readability and quality sdk-migration To fix this we need to migrate some code to the new SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants