Simplify getters by always throwing #347
Conversation
| } | ||
| } | ||
|
|
||
| private fun RNPlayerView.getPlayerViewOrThrow() = playerView |
There was a problem hiding this comment.
Nit: I quite like the require naming convention that androidx libraries use: https://developer.android.com/reference/kotlin/androidx/fragment/app/Fragment#requireActivity()
| private fun RNPlayerView.getPlayerViewOrThrow() = playerView | |
| private fun RNPlayerView.requirePlayerView() = playerView |
| @@ -46,6 +45,7 @@ class RNPlayerViewManager(private val context: ReactApplicationContext) : Simple | |||
| override fun getName() = MODULE_NAME | |||
|
|
|||
| private var customMessageHandlerBridgeId: NativeId? = null | |||
| private val handler = Handler(Looper.getMainLooper()) | |||
There was a problem hiding this comment.
| private val handler = Handler(Looper.getMainLooper()) | |
| private val mainHandler = Handler(Looper.getMainLooper()) |
| Handler(Looper.getMainLooper()).post { | ||
| view.playerView?.setFullscreenHandler( | ||
| context.getModule<FullscreenHandlerModule>()?.getInstance(fullscreenBridgeId), | ||
| handler.postAndLogException { |
There was a problem hiding this comment.
I think this is already in the develop branch, could you rebase this branch please?
| inline fun <reified T : ReactContextBaseJavaModule> ReactContext.getModule(): T? { | ||
| return getNativeModule(T::class.java) | ||
| } | ||
| private inline fun <reified T : ReactContextBaseJavaModule> ReactContext.getModuleOrThrow( |
There was a problem hiding this comment.
Nit: I quite like the require naming convention that androidx libraries use: https://developer.android.com/reference/kotlin/androidx/fragment/app/Fragment#requireActivity()
| private inline fun <reified T : ReactContextBaseJavaModule> ReactContext.getModuleOrThrow( | |
| private inline fun <reified T : ReactContextBaseJavaModule> ReactContext.requireModule( |
| } | ||
| private inline fun <reified T : ReactContextBaseJavaModule> ReactContext.getModuleOrThrow( | ||
| name: String, | ||
| ): T = getNativeModule(T::class.java).throwIfNull(name) |
There was a problem hiding this comment.
I don't think we need this extension function:
| ): T = getNativeModule(T::class.java).throwIfNull(name) | |
| ): T = getNativeModule(T::class.java) ?: error("${name}Module not found") |
There was a problem hiding this comment.
Mainly agree with the comments added by @zigavehovec.
Will have another look once the branch is in sync with the development branch :)
Problem (PRN-92)
Getters were duplicated between returning null or throwing. Now that Exception are consistently caught, those returning a nullable type can be removed.
Changes
Remove and inline many getters.
Accessing another module is now consistently and more transparently:
context.MODULE_NAME📚 Other PRs for this issue