Feature/about kotlin

[pfaffenrodt]

as promised the pull request of the kotlin version

[TheLastProject]

Thanks, I will take a look at this later. Given Kotlin is a pretty big step and we had a lot of new contributions Hacktoberfest I want to first do some final cleanup and checks and do a new release before starting this route so I can keep the releases somewhat small :)

[TheLastProject]

I generally like the changes here and the new style, but I have concerns about how your code prefers to silently do the wrong thing over crashing.

While crashes are obviously not nice to have, they at least show up in the Google Play Console. They let me know the app is misbehaving. By just returning if a precondition that should always be met isn't met, the app instead just misbehaves without me as developer ever knowing. I find that much worse.

[TheLastProject]

Could you explain what this does or link me to a good resource explaining it? I'm pretty new to Kotlin and the explanations I'm finding online don't really explain this well to me. Given I will have to maintain this code, I need to understand what it does.

[pfaffenrodt]

basicly, it will be the instance of the object. https://kotlinlang.org/docs/scope-functions.html#also

producing an instance has 'also' the sideeffect that the field variable is set.
coupling things

[TheLastProject]

Okay so:

Call AboutContent's constructor and assign whatever it gives back to the variable content. Then set this.content to the newly created content variable. Right?

I don't quite understand why that's cleaner than just directly doing:

this.content = AboutContent(this)

though. Is this some kind of best practice I don't know about?

[pfaffenrodt]

It is to use a not nullable variable. in the this scope it is nullable. You would unnecessarily check if it null, after instantiating

[TheLastProject]

Sorry for taking so long to get back to this. I've been thinking of this and I'm wondering, wouldn't it be cleaner if we remove the class level variables? That way we don't have to worry about manually nulling variables for garbage collection either and it severely simplifies the code:

Suggested change

	package protect.card_locker
	import android.os.Bundle
	import android.view.MenuItem
	import android.view.View
	import com.google.android.material.dialog.MaterialAlertDialogBuilder
	import protect.card_locker.databinding.AboutActivityBinding
	class AboutActivity : CatimaAppCompatActivity() {
	private var binding: AboutActivityBinding? = null
	private var content: AboutContent? = null
	override fun onCreate(savedInstanceState: Bundle?) {
	super.onCreate(savedInstanceState)
	val binding = AboutActivityBinding.inflate(layoutInflater)
	.also { this.binding = it }
	val content = AboutContent(this)
	.also { this.content = it }
	title = content.pageTitle
	setContentView(binding.root)
	setSupportActionBar(binding.toolbar)
	enableToolbarBackButton()
	val copyright = binding.creditsSub
	copyright.text = content.copyright
	val versionHistory = binding.versionHistorySub
	versionHistory.text = content.versionHistory
	binding.versionHistory.tag = "https://catima.app/changelog/"
	binding.translate.tag = "https://hosted.weblate.org/engage/catima/"
	binding.license.tag = "https://github.com/CatimaLoyalty/Android/blob/master/LICENSE"
	binding.repo.tag = "https://github.com/CatimaLoyalty/Android/"
	binding.privacy.tag = "https://catima.app/privacy-policy/"
	binding.reportError.tag = "https://github.com/CatimaLoyalty/Android/issues"
	binding.rate.tag = "https://play.google.com/store/apps/details?id=me.hackerchick.catima"
	bindClickListeners()
	}
	override fun onOptionsItemSelected(item: MenuItem): Boolean {
	val id = item.itemId
	if (id == android.R.id.home) {
	finish()
	}
	return super.onOptionsItemSelected(item)
	}
	override fun onDestroy() {
	super.onDestroy()
	content?.destroy()
	content = null
	clearClickListeners()
	binding = null
	}
	private fun bindClickListeners() {
	val binding = binding!!
	val openExternalBrowser = View.OnClickListener { view: View ->
	val tag = view.tag
	if (tag is String && tag.startsWith("https://")) {
	OpenWebLinkHandler().openBrowser(this, tag)
	}
	}
	binding.versionHistory.setOnClickListener(openExternalBrowser)
	binding.translate.setOnClickListener(openExternalBrowser)
	binding.license.setOnClickListener(openExternalBrowser)
	binding.repo.setOnClickListener(openExternalBrowser)
	binding.privacy.setOnClickListener(openExternalBrowser)
	binding.reportError.setOnClickListener(openExternalBrowser)
	binding.rate.setOnClickListener(openExternalBrowser)
	binding.credits.setOnClickListener { showCredits() }
	}
	private fun clearClickListeners() {
	val binding = binding!!
	binding.versionHistory.setOnClickListener(null)
	binding.translate.setOnClickListener(null)
	binding.license.setOnClickListener(null)
	binding.repo.setOnClickListener(null)
	binding.privacy.setOnClickListener(null)
	binding.reportError.setOnClickListener(null)
	binding.rate.setOnClickListener(null)
	binding.credits.setOnClickListener(null)
	}
	private fun showCredits() {
	val content = content!!
	MaterialAlertDialogBuilder(this)
	.setTitle(R.string.credits)
	.setMessage(content.contributorInfo)
	.setPositiveButton(R.string.ok, null)
	.show()
	}
	companion object {
	private const val TAG = "Catima"
	}
	}
	package protect.card_locker
	import android.os.Bundle
	import android.view.MenuItem
	import android.view.View
	import com.google.android.material.dialog.MaterialAlertDialogBuilder
	import protect.card_locker.databinding.AboutActivityBinding
	class AboutActivity : CatimaAppCompatActivity() {
	override fun onCreate(savedInstanceState: Bundle?) {
	super.onCreate(savedInstanceState)
	val binding = AboutActivityBinding.inflate(layoutInflater)
	val content = AboutContent(this)
	title = content.pageTitle
	setContentView(binding.root)
	setSupportActionBar(binding.toolbar)
	enableToolbarBackButton()
	val copyright = binding.creditsSub
	copyright.text = content.copyright
	val versionHistory = binding.versionHistorySub
	versionHistory.text = content.versionHistory
	binding.versionHistory.tag = "https://catima.app/changelog/"
	binding.translate.tag = "https://hosted.weblate.org/engage/catima/"
	binding.license.tag = "https://github.com/CatimaLoyalty/Android/blob/master/LICENSE"
	binding.repo.tag = "https://github.com/CatimaLoyalty/Android/"
	binding.privacy.tag = "https://catima.app/privacy-policy/"
	binding.reportError.tag = "https://github.com/CatimaLoyalty/Android/issues"
	binding.rate.tag = "https://play.google.com/store/apps/details?id=me.hackerchick.catima"
	bindClickListeners(binding, content)
	}
	override fun onOptionsItemSelected(item: MenuItem): Boolean {
	val id = item.itemId
	if (id == android.R.id.home) {
	finish()
	}
	return super.onOptionsItemSelected(item)
	}
	private fun bindClickListeners(binding: AboutActivityBinding, content: AboutContent) {
	val openExternalBrowser = View.OnClickListener { view: View ->
	val tag = view.tag
	if (tag is String && tag.startsWith("https://")) {
	OpenWebLinkHandler().openBrowser(this, tag)
	}
	}
	binding.versionHistory.setOnClickListener(openExternalBrowser)
	binding.translate.setOnClickListener(openExternalBrowser)
	binding.license.setOnClickListener(openExternalBrowser)
	binding.repo.setOnClickListener(openExternalBrowser)
	binding.privacy.setOnClickListener(openExternalBrowser)
	binding.reportError.setOnClickListener(openExternalBrowser)
	binding.rate.setOnClickListener(openExternalBrowser)
	binding.credits.setOnClickListener { showCredits(content) }
	}
	private fun showCredits(content: AboutContent) {
	MaterialAlertDialogBuilder(this)
	.setTitle(R.string.credits)
	.setMessage(content.contributorInfo)
	.setPositiveButton(R.string.ok, null)
	.show()
	}
	}

(I also seem to have accidentally made it so this PR needs rebasing, sorry)