search

facebookincubator / Battery-Metrics

Library that helps in instrumenting battery related system metrics.
MIT License
745 stars 79 forks source link

Remove synthetic accessor methods #9

Closed VisheshVadhera closed 7 years ago

VisheshVadhera commented 7 years ago

Closes #8

Summary: Changed the visibility modifier of members from private to package private. This won't lead to synthetic accessor methods getting created for accessing the members (which were earlier private) inside the anonymous classes.

kunalb commented 7 years ago

Thanks! do you know if there's any good annotation that identifies Visible-to-avoid-synthetic-accessors? I can imagine someone refactoring this back to private if they don't pay attention.

simpleton commented 7 years ago

@kunalb Maybe u can enable Synthetic accessor call in inspections settings, even change it's severity to error.

VisheshVadhera commented 7 years ago

@kunalb AOSP Launcher 3 uses an annotation to signal that the member is package private only to prevent creation of synthetic accessors: https://android.googlesource.com/platform/packages/apps/Launcher3/+/master/src/com/android/launcher3/util/Thunk.java

Do you feel we can use something similar for this project as well?

kunalb commented 7 years ago

Yeah, I'd like that a lot – except I'm not a fan of the name "Thunk" – maybe a source annotation called @VisibleToAvoidSynthetics? (Similar to VisibleForTesting)

VisheshVadhera commented 7 years ago

Yeah, that is certainly better than "Thunk". I'll open a PR tonight.

Also a good thing (not right away but in future) would be to add a lint check to the build process to fail the build if at all new synthetic accessors get into the project either due to refactoring or due to new code.