firebase / snippets-android

Android snippets for firebase.google.com
Apache License 2.0
769 stars 403 forks source link

Use implicit name instead of `user` #421

Closed peterfriese closed 1 year ago

thatfiredev commented 1 year ago

I think the reason why we don't use it is to make it easier to understand where the data is coming from for folks not familiar with Kotlin.

A different suggestion would be to use a different name for the first variable (eg. firebaseUser):

val firebaseUser = Firebase.auth.currentUser
firebaseUser?.let { user ->
    // Name, email address, and profile photo Url
    val name = user.displayName
    val email = user.email
    val photoUrl = user.photoUrl

}
peterfriese commented 1 year ago

I think the reason why we don't use it is to make it easier to understand where the data is coming from for folks not familiar with Kotlin.

Good point. I checked the rest of the file, and we do use it in https://github.com/firebase/snippets-android/blob/ca389d977517f17f579c57f058380d2b1e9e733b/auth/app/src/main/java/com/google/firebase/quickstart/auth/kotlin/MainActivity.kt#L77 . Isn't that inconsistent?

A different suggestion would be to use a different name for the first variable (eg. firebaseUser):

val firebaseUser = Firebase.auth.currentUser
firebaseUser?.let { user ->
    // Name, email address, and profile photo Url
    val name = user.displayName
    val email = user.email
    val photoUrl = user.photoUrl

}

Generally speaking, it feels cleaner to me to not access a variable from the outer scope if the same variable is also accessible on the inside of the closure. I would assume that the compiled code looks more or less the same, but from the reader's perspective, it feels different.