Skip to content
This repository was archived by the owner on Jan 9, 2026. It is now read-only.

Reduce amount of code#443

Merged
nic0lette merged 3 commits into
android:mainfrom
GeorgCantor:main
Apr 8, 2021
Merged

Reduce amount of code#443
nic0lette merged 3 commits into
android:mainfrom
GeorgCantor:main

Conversation

@GeorgCantor
Copy link
Copy Markdown
Contributor

Reduced the amount of code using Kotlin Synthetic

@nic0lette
Copy link
Copy Markdown
Contributor

Hi @GeorgCantor ! Thanks for the PR.

Since Kotlin Android Extensions are deprecated, we don't want to use Kotlin synthetics here. If you were able to migrate these to use view binding, then I'd be able to accept the PR.

@GeorgCantor
Copy link
Copy Markdown
Contributor Author

Ok

Copy link
Copy Markdown
Contributor

@nic0lette nic0lette left a comment

Choose a reason for hiding this comment

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

Thanks for the update! Just a few small things but otherwise looks good =D


override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
binding = QrSignInBinding.bind(view)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you make binding a local variable then it wouldn't be nullable (so you could get rid of many of the ?s) and you wouldn't need to clear it in onDestroy()

*/
private fun setQrCode(url: String) {
Glide.with(this).load(url).into(qr_code)
binding?.qrCode?.let { Glide.with(this).load(url).into(it) }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you just pass the view in with the url (or, perhaps even better, just move it into onViewCreated())?

*/
class SettingsActivity : AppCompatActivity() {

private lateinit var binding: ActivitySettingsBinding
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this need to be a member? It looks like everything is set in onCreate?

Copy link
Copy Markdown
Contributor

@nic0lette nic0lette left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@nic0lette nic0lette merged commit 99e44c1 into android:main Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants