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

Migrate a lot of code to use BaseItemDto from SDK #2081

Merged
merged 2 commits into from
Sep 23, 2022

Conversation

nielsvanvelzen
Copy link
Member

I spend like 5 hours on this

Changes
Use BaseItemDto from SDK in BaseRowItem internally and fix everything that (in)directly depends on it... that's quite a lot

Some functions that were added to the BaseItemDto in the legacy apiclient have been copied and rewritten (partially in JavaCompat and sometimes as private methods in Java classes).

This PR should not change behavior.

Issues

@nielsvanvelzen nielsvanvelzen added the refactor Improvements to code realiability, readability and quality label Sep 20, 2022
@nielsvanvelzen nielsvanvelzen added this to the v0.15.0 milestone Sep 20, 2022

open class BaseRowItem private constructor(
open class BaseRowItem protected constructor(

Check warning

Code scanning / detekt

Too many functions inside a/an file/class/object/interface always indicate a violation of the single responsibility principle. Maybe the file/class/object/interface wants to manage too many things at once. Extract functionality which clearly belongs together.

Class 'BaseRowItem' with '17' functions detected. Defined threshold inside classes is set to '15'
@@ -269,23 +278,23 @@
val start = baseItem?.startDate
val end = baseItem?.endDate

if (start != null && end != null) (end.time - start.time) * 10000
if (start != null && end != null) (start.until(end, ChronoUnit.MILLIS)) * 10000

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
item?.type == BaseItemKind.MOVIE && item.criticRating != null -> when {
item.criticRating!! > 59f -> R.drawable.ic_rt_fresh
baseItem?.type == BaseItemKind.MOVIE && baseItem!!.criticRating != null -> when {
baseItem!!.criticRating!! > 59f -> R.drawable.ic_rt_fresh

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
val placeholder = ContextCompat.getDrawable(context, R.drawable.ic_album)
val blurHash = item.imageBlurHashes?.get(ImageType.Primary)?.get(item.imageTags?.get(ImageType.Primary))
val blurHash = item.imageBlurHashes?.get(org.jellyfin.sdk.model.api.ImageType.PRIMARY)?.get(item.imageTags?.get(org.jellyfin.sdk.model.api.ImageType.PRIMARY))

Check warning

Code scanning / detekt

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

Line detected, which is longer than the defined maximum line length in the code style.
@@ -80,7 +78,7 @@
}

private var audioEventListener: AudioEventListener = object : AudioEventListener {
override fun onPlaybackStateChange(newState: PlaybackController.PlaybackState, currentItem: BaseItemDto?) {
override fun onPlaybackStateChange(newState: PlaybackController.PlaybackState, currentItem: org.jellyfin.sdk.model.api.BaseItemDto?) {

Check warning

Code scanning / detekt

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

Line detected, which is longer than the defined maximum line length in the code style.
binding.npIcon.load(ImageUtils.getPrimaryImageUrl(item), blurHash, placeholder, item.primaryImageAspectRatio ?: 1.0)

currentDuration = TimeUtils.formatMillis(if (item.runTimeTicks != null) item.runTimeTicks / 10_000 else 0)
currentDuration = TimeUtils.formatMillis(if (item.runTimeTicks != null) item.runTimeTicks!! / 10_000 else 0)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
@nielsvanvelzen nielsvanvelzen marked this pull request as ready for review September 20, 2022 16:34
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 61 potential problems in the proposed changes. Check the Files changed tab for more details.

@nielsvanvelzen nielsvanvelzen merged commit 135645f into jellyfin:master Sep 23, 2022
@nielsvanvelzen nielsvanvelzen deleted the sdk-baseitemdto branch September 23, 2022 20:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant