Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use strong class reference for no-config glue class #32

Merged
merged 1 commit into from
Apr 3, 2020

Conversation

LukasPaczos
Copy link

Closes #31.

Instead of storing String references to the class, we can actually import the implementation class which not only saves us one reflection class lookup, but also prevents minification of the implementation class thanks to a binary reference.

@LukasPaczos LukasPaczos requested review from tobrun and RingerJK April 2, 2020 13:05

@JvmStatic
val implClassName: String = "HttpClientImpl"
val implClass: Class<HttpClientImpl> = HttpClientImpl::class.java
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be?

Suggested change
val implClass: Class<HttpClientImpl> = HttpClientImpl::class.java
val implClass: Class<com.mapbox.maps.module.http.HttpClientImpl> = com.mapbox.maps.module.http.HttpClientImpl::class.java

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the generated class will have an import statement as well. This is just an example.

Copy link

@RingerJK RingerJK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@LukasPaczos LukasPaczos merged commit c4824a9 into master Apr 3, 2020
@LukasPaczos LukasPaczos deleted the lp-strong-class-reference branch April 3, 2020 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid module implementation minification
3 participants