Convert language related classes to Kotlin#175
Conversation
WalkthroughThis pull request refactors the language selection functionality by removing the previous Java implementations and introducing new Kotlin-based versions. The original Changes
Sequence Diagram(s)sequenceDiagram
participant S as SelectLanguageActivity
participant F as LanguageListDialogFragment
participant A as LanguageAdapter
participant M as MainActivity
S->>F: onStart calls newInstance() to display dialog
F->>A: Initialize RecyclerView & adapter
A->>F: On language item click (log & persist selection)
F->>M: Launch MainActivity and finish current activity
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
app/src/main/java/ai/elimu/content_provider/language/SelectLanguageActivity.kt (2)
17-22: Consider adding dialog dismissal handling.The dialog fragment is shown in onStart(), which is correct, but there's no handling to prevent showing multiple instances if onStart() is called multiple times (like when returning from background). Consider checking if the dialog is already showing before creating a new instance.
override fun onStart() { Log.i(javaClass.name, "onStart") super.onStart() - newInstance().show(supportFragmentManager, "dialog") + if (supportFragmentManager.findFragmentByTag("dialog") == null) { + newInstance().show(supportFragmentManager, "dialog") + } }
4-4: Use standard Kotlin import convention.Consider using a direct import instead of importing the companion object method. This is more idiomatic in Kotlin.
-import ai.elimu.content_provider.language.LanguageListDialogFragment.Companion.newInstance +import ai.elimu.content_provider.language.LanguageListDialogFragmentThen update the usage:
-newInstance().show(supportFragmentManager, "dialog") +LanguageListDialogFragment.newInstance().show(supportFragmentManager, "dialog")app/src/main/java/ai/elimu/content_provider/language/LanguageListDialogFragment.kt (6)
74-74: Use Kotlin best practices for enum collection handling.Instead of
.entries.toTypedArray(), you can directly use.values()for enum values in Kotlin. This is more idiomatic and avoids unnecessary conversions.-private val languages = Language.entries.toTypedArray() +private val languages = Language.values()
83-95: Use Kotlin's lambda syntax for click listeners.Replace the anonymous
View.OnClickListenerwith Kotlin's lambda syntax for a more concise and idiomatic implementation.-holder.text.setOnClickListener(object : View.OnClickListener { - override fun onClick(v: View) { - Log.i(javaClass.name, "onClick") - Log.i(javaClass.name, "language: $language") - SharedPreferencesHelper.storeLanguage(context, language) - val mainActivityIntent = Intent( - context, - MainActivity::class.java - ) - startActivity(mainActivityIntent) - activity!!.finish() - } -}) +holder.text.setOnClickListener { + Log.i(javaClass.name, "onClick") + Log.i(javaClass.name, "language: $language") + SharedPreferencesHelper.storeLanguage(context, language) + val mainActivityIntent = Intent( + context, + MainActivity::class.java + ) + startActivity(mainActivityIntent) + activity?.finish() +}
103-109: Simplify companion object function.The
newInstance()function can be simplified since it's not using any parameters.companion object { @JvmStatic fun newInstance(): LanguageListDialogFragment { - val fragment = LanguageListDialogFragment() - return fragment + return LanguageListDialogFragment() } }
41-53: Consider implementing view binding.Using view binding would make the code safer by eliminating the need for casting and findViewById calls.
First, enable view binding in your build.gradle:
android { ... buildFeatures { viewBinding true } }Then update your code to use view binding:
private var _binding: FragmentLanguageListDialogListDialogBinding? = null private val binding get() = _binding!! override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View { _binding = FragmentLanguageListDialogListDialogBinding.inflate(inflater, container, false) return binding.root } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { binding.recyclerView.layoutManager = LinearLayoutManager(context) binding.recyclerView.adapter = LanguageAdapter() binding.recyclerView.addItemDecoration( DividerItemDecoration( context, DividerItemDecoration.VERTICAL ) ) isCancelable = false } override fun onDestroyView() { super.onDestroyView() _binding = null }
19-27: Update documentation style.The KDoc style documentation is using JavaDoc format with HTML tags. Consider updating it to use proper KDoc format.
/** - * - * A fragment that shows a list of items as a modal bottom sheet. - * - * You can show this modal bottom sheet from your activity like this: - * <pre> - * LanguageListDialogFragment.newInstance().show(getSupportFragmentManager(), "dialog"); - *</pre> * + * A fragment that shows a list of items as a modal bottom sheet. + * + * You can show this modal bottom sheet from your activity like this: + * ``` + * LanguageListDialogFragment.newInstance().show(supportFragmentManager, "dialog") + * ``` */
88-91: Simplify Intent creation.Use Kotlin's simplified syntax for creating Intents.
-val mainActivityIntent = Intent( - context, - MainActivity::class.java -) +val mainActivityIntent = Intent(context, MainActivity::class.java)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/ai/elimu/content_provider/language/LanguageListDialogFragment.java(0 hunks)app/src/main/java/ai/elimu/content_provider/language/LanguageListDialogFragment.kt(1 hunks)app/src/main/java/ai/elimu/content_provider/language/SelectLanguageActivity.java(0 hunks)app/src/main/java/ai/elimu/content_provider/language/SelectLanguageActivity.kt(1 hunks)
💤 Files with no reviewable changes (2)
- app/src/main/java/ai/elimu/content_provider/language/SelectLanguageActivity.java
- app/src/main/java/ai/elimu/content_provider/language/LanguageListDialogFragment.java
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: build (ubuntu-latest, 17)
🔇 Additional comments (2)
app/src/main/java/ai/elimu/content_provider/language/SelectLanguageActivity.kt (1)
9-15: Lifecycle handling looks good.The activity properly overrides onCreate, calls super.onCreate and sets the content view layout.
app/src/main/java/ai/elimu/content_provider/language/LanguageListDialogFragment.kt (1)
87-87: Consider handling potential null context.When storing the language preference, you're using the context directly which could be null. Consider using a safe call or requireContext() to handle this case properly.
-SharedPreferencesHelper.storeLanguage(context, language) +SharedPreferencesHelper.storeLanguage(requireContext(), language)
| MainActivity::class.java | ||
| ) | ||
| startActivity(mainActivityIntent) | ||
| activity!!.finish() |
There was a problem hiding this comment.
Replace unsafe null assertion with safe call.
The use of !! can lead to NullPointerExceptions. Replace with the safe call operator ?. for better null safety.
-activity!!.finish()
+activity?.finish()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| activity!!.finish() | |
| activity?.finish() |
There was a problem hiding this comment.
@tuancoltech Thanks for making that change! Using safe call operators is a great practice in Kotlin for better null safety.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
2 participants
Convert language related classes to Kotlin
Summary by CodeRabbit
New Features
Refactor