android / codelab-exoplayer-intro

Media Streaming with ExoPlayer codelab
https://codelabs.developers.google.com/codelabs/exoplayer-intro
Apache License 2.0
221 stars 116 forks source link

Update kotlin to be more idiomatic #62

Closed tunjid closed 3 years ago

tunjid commented 3 years ago

Updates the kotlin to be more idiomatic (gets rid of the !! force null unwrapping mostly).

I favored more descriptive variable names over more idiomatic lambdas for beginner accessibility.

For example:

    private fun releasePlayer() {
        val nullablePlayer = player
        if (nullablePlayer != null) {
            playbackPosition = nullablePlayer.currentPosition
            currentWindow = nullablePlayer.currentWindowIndex
            playWhenReady = nullablePlayer.playWhenReady
            nullablePlayer.release()
            player = null
        }
    }

versus

        player?.let {
            playbackPosition = it.currentPosition
            currentWindow = it.currentWindowIndex
            playWhenReady = it.playWhenReady
            it.release()
        }
        player = null
    }

Let me know what you think.

dturner commented 3 years ago

Yes, this is a more idiomatic approach. Did you want to update this PR with your suggested changes?

tunjid commented 3 years ago

@dturner I should use the let method over the extra declared variable?

dturner commented 3 years ago

Sorry, I should've been clearer. Yes the let method is much clearer. Also, regarding the first method, I would avoid any variable which includes type information in its name e.g. nullablePlayer since the nullability is inferred from its type.

dturner commented 3 years ago

Would it be possible to merge all these PRs into a single PR? At the moment it's difficult to see exactly what's changed from the original.

tunjid commented 3 years ago

@dturner will do!

tunjid commented 3 years ago

Closing in favor of #63