Convert WordAssessmentActivity to Kotlin#54
Conversation
WalkthroughThis pull request introduces Kotlin enhancements by updating the build configuration and refactoring an activity. The Changes
Possibly related PRs
Suggested reviewers
Poem
📜 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 (6)
🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
app/src/main/java/ai/elimu/kukariri/assessment/WordAssessmentActivity.kt (7)
23-40: Consider using lateinit instead of nullable types for viewsThe UI components are currently defined as nullable types with initialization deferred to onCreate. For views that are guaranteed to be initialized before use, Kotlin's
lateinitmodifier would be more idiomatic and eliminate the need for null-safety operators.- private var progressBar: ProgressBar? = null - private var textView: TextView? = null - private var difficultButton: Button? = null - private var easyButton: Button? = null + private lateinit var progressBar: ProgressBar + private lateinit var textView: TextView + private lateinit var difficultButton: Button + private lateinit var easyButton: ButtonAlso, for the collections, consider using Kotlin's collection factory functions:
- private val wordGsonsPendingReview: MutableList<WordGson> = ArrayList() - private val wordGsonsMastered: MutableList<WordGson> = ArrayList() + private val wordGsonsPendingReview: MutableList<WordGson> = mutableListOf() + private val wordGsonsMastered: MutableList<WordGson> = mutableListOf()
86-91: Simplify word type filtering with Kotlin's 'in' operatorThe multi-line conditional check for word types could be simplified using Kotlin's
inoperator with a set of word types.- if ((wordGson.wordType == WordType.ADJECTIVE) - || (wordGson.wordType == WordType.NOUN) - || (wordGson.wordType == WordType.VERB) - ) { + if (wordGson.wordType in setOf(WordType.ADJECTIVE, WordType.NOUN, WordType.VERB)) { wordGsonsPendingReview.add(wordGson) }
117-118: Use property assignment instead of setter method for durationIn Kotlin, you can directly assign values to properties rather than using setter methods.
- objectAnimator.setDuration(1000) + objectAnimator.duration = 1000
122-125: Eliminate null assertion operators with lateinitIf you change the TextView to use lateinit as suggested earlier, you can remove the null assertion operators (!!) which improves code safety.
- textView!!.text = wordGson.text val appearAnimation = AnimationUtils.loadAnimation(applicationContext, R.anim.anim_appear_right) - textView!!.startAnimation(appearAnimation) + textView.text = wordGson.text + textView.startAnimation(appearAnimation)
133-136: Use string templates and safer null handlingKotlin offers string templates for more readable string concatenation, and you can use safe call operators to handle potential null values.
- textView!!.text = textView!!.text.toString() + "\n" - for (emojiGson in emojiGsons) { - textView!!.text = textView!!.text.toString() + emojiGson.glyph - } + textView.text = "${textView.text}\n" + for (emojiGson in emojiGsons) { + textView.text = "${textView.text}${emojiGson.glyph}" + }
141-157: Replace anonymous inner class with lambda expressionKotlin allows for more concise click listeners using lambda expressions instead of anonymous inner classes.
- difficultButton!!.setOnClickListener(object : View.OnClickListener { - override fun onClick(v: View) { - Log.i(javaClass.name, "difficultButton onClick") - - // Move the Word to the end of the list - wordGsonsPendingReview.remove(wordGson) - wordGsonsPendingReview.add(wordGson) - - // Report assessment event to the Analytics application (https://github.com/elimu-ai/analytics) - AssessmentEventUtil.reportWordAssessmentEvent( - wordGson, 0.00f, System.currentTimeMillis() - timeStart, - applicationContext, BuildConfig.ANALYTICS_APPLICATION_ID - ) - - loadNextWord() - } - }) + difficultButton.setOnClickListener { + Log.i(javaClass.name, "difficultButton onClick") + + // Move the Word to the end of the list + wordGsonsPendingReview.remove(wordGson) + wordGsonsPendingReview.add(wordGson) + + // Report assessment event to the Analytics application (https://github.com/elimu-ai/analytics) + AssessmentEventUtil.reportWordAssessmentEvent( + wordGson, 0.00f, System.currentTimeMillis() - timeStart, + applicationContext, BuildConfig.ANALYTICS_APPLICATION_ID + ) + + loadNextWord() + }
159-175: Replace anonymous inner class with lambda expressionSimilar to the previous comment, this click listener can also be simplified with a lambda expression.
- easyButton!!.setOnClickListener(object : View.OnClickListener { - override fun onClick(v: View) { - Log.i(javaClass.name, "easyButton onClick") - - // Remove the Word from the list of Words to be repeated, and add it to the list of mastered Words - wordGsonsPendingReview.remove(wordGson) - wordGsonsMastered.add(wordGson) - - // Report assessment event to the Analytics application (https://github.com/elimu-ai/analytics) - AssessmentEventUtil.reportWordAssessmentEvent( - wordGson, 1.00f, System.currentTimeMillis() - timeStart, - applicationContext, BuildConfig.ANALYTICS_APPLICATION_ID - ) - - loadNextWord() - } - }) + easyButton.setOnClickListener { + Log.i(javaClass.name, "easyButton onClick") + + // Remove the Word from the list of Words to be repeated, and add it to the list of mastered Words + wordGsonsPendingReview.remove(wordGson) + wordGsonsMastered.add(wordGson) + + // Report assessment event to the Analytics application (https://github.com/elimu-ai/analytics) + AssessmentEventUtil.reportWordAssessmentEvent( + wordGson, 1.00f, System.currentTimeMillis() - timeStart, + applicationContext, BuildConfig.ANALYTICS_APPLICATION_ID + ) + + loadNextWord() + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/libs.versions.tomlis excluded by!**/*.toml
📒 Files selected for processing (3)
app/build.gradle(3 hunks)app/src/main/java/ai/elimu/kukariri/assessment/WordAssessmentActivity.java(0 hunks)app/src/main/java/ai/elimu/kukariri/assessment/WordAssessmentActivity.kt(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/ai/elimu/kukariri/assessment/WordAssessmentActivity.java
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 21)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: build (macos-latest, 17)
🔇 Additional comments (4)
app/build.gradle (3)
1-3: Well-structured Kotlin Android plugin integrationThe addition of the Kotlin Android plugin is correctly implemented using the version catalog approach, which is a modern best practice for managing plugin versions in Gradle.
50-52: Appropriate Kotlin JVM target configurationThe kotlinOptions block correctly sets the JVM target to '17', which matches the Java source compatibility defined in the compileOptions block. This ensures consistent bytecode generation across Java and Kotlin sources.
70-70: Good addition of androidx.core.ktx dependencyThe androidx.core.ktx library provides Kotlin extensions for AndroidX components, making Android development with Kotlin more concise and idiomatic. This is an essential dependency for Kotlin Android projects.
app/src/main/java/ai/elimu/kukariri/assessment/WordAssessmentActivity.kt (1)
1-177: Successfully converted Java activity to Kotlin with functional equivalenceThe WordAssessmentActivity has been successfully converted from Java to Kotlin while maintaining the same functionality. The code handles word assessment logic, UI interactions, and event logging appropriately.
While there are suggestions for making the code more idiomatic Kotlin (as noted in other comments), the current implementation is functionally correct and follows the basic principles of Kotlin programming.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/libs.versions.tomlis excluded by!**/*.toml
📒 Files selected for processing (2)
app/build.gradle(3 hunks)build.gradle(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/build.gradle
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: build (macos-latest, 21)
| dependencies { | ||
| classpath 'com.android.tools.build:gradle:8.5.2' | ||
| classpath 'com.mxalbert.gradle:jacoco-android:0.2.1' | ||
| classpath libs.kotlin.gradle.plugin |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Kotlin Gradle Plugin Dependency Addition
The addition of the Kotlin Gradle plugin via classpath libs.kotlin.gradle.plugin is a crucial change towards enabling Kotlin support. Please ensure that the libs.kotlin.gradle.plugin reference is correctly defined (e.g., in a version catalog like libs.versions.toml) and corresponds to the desired Kotlin version for the project. If not, consider explicitly specifying the version to avoid resolution issues.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
============================================
- Coverage 11.55% 10.84% -0.71%
Complexity 9 9
============================================
Files 10 10
Lines 199 212 +13
Branches 37 37
============================================
Hits 23 23
- Misses 175 188 +13
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@jo-elimu It seems |
@tuancoltech As far as I know, none of the Kotlin files are covered by unit tests?: https://app.codecov.io/gh/elimu-ai/kukariri/tree/main/app%2Fsrc%2Fmain%2Fjava%2Fai%2Felimu%2Fkukariri%2Flogic I guess |
@jo-elimu The |
|
@tuancoltech I'm not 100% sure, but yes, I think codecov status checks fail if the overall coverage percentage goes down. And show as passed (green) whenever the coverage percentage remains unchanged or increases. |
@jo-elimu Thanks. Let me just merge this PR for now then. |
Convert WordAssessmentActivity to Kotlin