google-developer-training / android-kotlin-fundamentals-apps

android-kotlin-fundamentals-apps
Other
1.68k stars 2.12k forks source link

Android Kotlin Fundamentals codelab: Confusing use of arguments property in example code #642

Closed chocobochicken closed 1 year ago

chocobochicken commented 3 years ago

Name of the Codelab or Codelab URL Android Kotlin Fundamentals: Use LiveData to control button states https://developer.android.com/codelabs/kotlin-android-training-quality-and-states#3

Describe the problem Example code fails to build. In trying to declare and assign a new "arguments" variable, it references this same variable name from within the assignment expression.

val arguments = SleepQualityFragmentArgs.fromBundle(arguments!!)

Naturally, arguments cannot be referenced in fromBundle if it has not yet been assigned. I assumed this may just have meant the savedInstanceState Bundle arg received from onCreateView, but trying that gave me a runtime NPE because the bundle was null, so I'm not sure what I'm doing wrong. The incorrect example code needs to be replaced with code which results in a working build.

In which lesson and step of the codelab can this issue be found? Lesson 4, Step 2, Bullet Point 2

How to reproduce?

  1. Open codelab lesson in a browser: https://developer.android.com/codelabs/kotlin-android-training-quality-and-states#3
  2. Verify the arguments assignment error present in Step 2, Bullet Point 2

Versions

  1. What version of Android Studio are you using? 4.1.3
  2. What API level are you targeting? 30

Additional information

codelab: android-kotlin-fundamentals

chocobochicken commented 3 years ago

Upon further inspection from the solution, it looks like arguments does compile, because it's not technically a self-reference, but instead the declared local variable arguments is independent in scope from the inherited Fragment property arguments, which in turn is interpreted from the Java base class getArguments() method. However, I respectfully suggest the obfuscation of an inherited property is confusing and should be reconsidered.

Additionally, Android Studio flags this with a high level warning recommending to replace the arguments parameter with the requireArguments() method instead. This syntax warning was why I read this code as a compile error.

I recommend updating the example/solution code to:

  1. Rename the local variable from arguments to sleepQualityArgs, so as not to obfuscate the inherited property
  2. Apply the recommended replacement from the arguments!! parameter to the requireArguments() method.