search

VladimirWrites / AnalogWatchFace

⌚ Analog Watch Face for Wear OS
https://vladimirj.dev/analog-watch-face/
Apache License 2.0
125 stars 21 forks source link

Android Lint #3

Closed Domi04151309 closed 4 years ago

Domi04151309 commented 4 years ago

This pull request applies optimizations suggested by the linter built into Android Studio.

VladimirWrites commented 4 years ago

Hi Dominik! Thank you very much for your contribution. There are some changes that I am comfortable with merging, but there are some that I would like to discuss with you. Let me do it commit by commit:

  1. Apply suggestions of category "Kotlin"

    • I don't like the idea of specifying the type for every variable. It seems redundant, and it creates a lot of noise. So I would rather update the lint rules.
    • Replacing *COMPLICATION_SUPPORTED_TYPES[complicationLocation.id]!! with *COMPLICATION_SUPPORTED_TYPES.getValue(complicationLocation.id) seems like a walid improvement. 👍
    • I don't like replacing setComplication(complicationProviderInfo, selectedComplication!!.id) with setComplication(complicationProviderInfo, selectedComplication?.id ?: return). I want the app to crash if selectedComplication is null because that shouldn't happen. I don't know how to handle that. Making this change would just hide potential problems.
    • I am intentionally leaving things non-private in tests because everything should be private in test classes (there should be no dependencies between tests), but that would just create additional noise in all test classes. But again, I should have created lint rules for this :(.

  2. Remove unused resources

    • wear/src/main/res/layout/activity_face_picker.xml should not be deleted. There is a mistake in the FacePickerActivity. setContentView(R.layout.activity_config) should be replaced with setContentView(R.layout.activity_face_picker).

  3. Prevent creation of synthetic accessor

    • I don't understand what's triggering the creation of synthetic accessor. Could you elaborate on this?

If you could update these things I would be happy to merge it.

Again, thanks a lot for your contribution!

Domi04151309 commented 4 years ago

Thanks for your suggestions. Here is an article about synthetic accessors. In short: Accessing private methods or fields in Java from nested or anonymous inner classes results in the creation of synthetic accessor methods.

Furthermore, I wanted to add that the use of Dependencies.kt makes it hard to keep track of newer versions.

VladimirWrites commented 4 years ago

Oh, I missed that the adapter is being accessed inside ProviderInfoRetriever.OnProviderInfoReceivedCallback object. Dependencies.kt helps a lot in a multi-module setup. But you are right, in this case, it doesn't help.