Conversation
|
1659 |
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Looks good but needs a few minor fixes 👍
| ) | ||
| } | ||
|
|
||
| private fun extractVideoId(url: String): String? { |
There was a problem hiding this comment.
The Youtube extractor already has this logic, but much better support for various links.
I suggest you remove extractVideoId and let the extractor handle the ID logic.
| return newHomePageResponse( | ||
| listOf( | ||
| HomePageList( | ||
| request.name.ifEmpty { "Trending" }, |
There was a problem hiding this comment.
This isEmpty should never be true, you can remove it.
|
|
||
| extractor.forceContentCountry(ContentCountry(Locale.getDefault().country)) | ||
|
|
||
| val pageData = if (page == 1) { |
There was a problem hiding this comment.
Please add a comment explaining the 'linked list logic' briefly. It is good practice to make it easy to maintain.
Something like: "To fetch the next page we must have a reference to the current page. To do that we store a cache of the current page." would work well.
fire-light42
left a comment
fire-light42
left a comment
There was a problem hiding this comment.
Almost ready for merge
| // Make mainPage dynamic and updatable | ||
| override var mainPage: List<MainPageData> = emptyList() | ||
|
|
||
| init { |
There was a problem hiding this comment.
I understand the logic behind placing this inside init, however you should not do it.
Setting mainPage in a thread during init will lead to race conditions where the mainPage is read before it is set.
Running potentially crashing code inside init can also lead to unexpected exceptions.
What you should do instead is move this logic to inside getMainPage, but inside getMainPage do not set the mainPage. If you just return a HomePageResponse with the appropriate rows it will work.
|
Any update for this PR? |
3 participants
bumps gradle plugin version from 2.1.0 to 2.3.0
Adds Youtube Provider
1, we can delete/ disable invidios app api's are locked
2, will implement channel, playlist as TvSeries Later
4, login and personalisation to be done later
3, rest all other functionality works fine