Skip to content

Commit

Permalink
Fix#3146 : Create a generic utility for filtering enums (#5529)
Browse files Browse the repository at this point in the history
## Explanation

Fixes: #3146

This PR introduces a new utility function filterByEnumCondition to
standardize filtering of enums across various parts of the oppia-android
codebase. This utility function allows filtering of collections based on
a condition applied to enum values.

This PR introduces a new utility function filterByEnumCondition to
standardize filtering of enums across various parts of the oppia-android
codebase. This utility function allows filtering of collections based on
a condition applied to enum values.

**Key Changes**:
Added Utility Function:
filterByEnumCondition: A generic function to filter a collection based
on a condition applied to an enum extracted from each item in the
collection.

**Updated Existing Code**:
Refactored code in getLeastRecentMetricLogIndex,
getLeastRecentMediumPriorityEventIndex, and other methods to utilize the
new filterByEnumCondition function.
Updated the calculation of completedChapterCount and
inProgressChapterCount using the new utility function.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [ x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x ] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x ] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x ] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [ x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x ] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).
-

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
  • Loading branch information
whyash8 and adhiamboperes authored Nov 16, 2024
1 parent 8a7fe0a commit 33d2b8f
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 58 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
# Utilities that are primarily used for frontend/UI purposes.
/utility/src/*/java/org/oppia/android/util/accessibility/ @oppia/android-frontend-reviewers
/utility/src/*/java/org/oppia/android/util/statusbar/ @oppia/android-frontend-reviewers
/utility/src/main/java/org/oppia/android/util/enumfilter/ @oppia/android-frontend-reviewers
/utility/src/*/java/org/oppia/android/util/extensions/ @oppia/android-frontend-reviewers
/utility/src/*/java/org/oppia/android/util/parser/html @oppia/android-frontend-reviewers
/utility/src/*/res/**/*.xml @oppia/android-frontend-reviewers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.oppia.android.app.survey.PreviousAnswerHandler
import org.oppia.android.app.survey.SelectedAnswerAvailabilityReceiver
import org.oppia.android.app.survey.SelectedAnswerHandler
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.enumfilter.filterByEnumCondition
import javax.inject.Inject

/** [SurveyAnswerItemViewModel] for the market fit question options. */
Expand Down Expand Up @@ -98,8 +99,12 @@ class MarketFitItemsViewModel @Inject constructor(
private fun getMarketFitOptions(): ObservableList<MultipleChoiceOptionContentViewModel> {
val appName = resourceHandler.getStringInLocale(R.string.app_name)
val observableList = ObservableArrayList<MultipleChoiceOptionContentViewModel>()
observableList += MarketFitAnswer.values()
.filter { it.isValid() }
val filteredmarketFitAnswer = filterByEnumCondition(
MarketFitAnswer.values().toList(),
{ marketFitAnswer -> marketFitAnswer },
{ marketFitAnswer -> marketFitAnswer.isValid() }
)
observableList += filteredmarketFitAnswer
.mapIndexed { index, marketFitAnswer ->
when (marketFitAnswer) {
MarketFitAnswer.VERY_DISAPPOINTED -> MultipleChoiceOptionContentViewModel(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.oppia.android.app.survey.SelectedAnswerAvailabilityReceiver
import org.oppia.android.app.survey.SelectedAnswerHandler
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ObservableArrayList
import org.oppia.android.util.enumfilter.filterByEnumCondition
import javax.inject.Inject

/** [SurveyAnswerItemViewModel] for providing the type of user question options. */
Expand Down Expand Up @@ -97,46 +98,36 @@ class UserTypeItemsViewModel @Inject constructor(

private fun getUserTypeOptions(): ObservableArrayList<MultipleChoiceOptionContentViewModel> {
val observableList = ObservableArrayList<MultipleChoiceOptionContentViewModel>()
observableList += UserTypeAnswer.values()
.filter { it.isValid() }
.mapIndexed { index, userTypeOption ->
when (userTypeOption) {
UserTypeAnswer.LEARNER ->
MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(
R.string.user_type_answer_learner
),
index,
this
)
UserTypeAnswer.TEACHER -> MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(
R.string.user_type_answer_teacher
),
index,
this
)

UserTypeAnswer.PARENT ->
MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(
R.string.user_type_answer_parent
),
index,
this
)

UserTypeAnswer.OTHER ->
MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(
R.string.user_type_answer_other
),
index,
this
)
else -> throw IllegalStateException("Invalid UserTypeAnswer")
}
val filteredUserTypes = filterByEnumCondition(
UserTypeAnswer.values().toList(),
{ userTypeAnswer -> userTypeAnswer },
{ userTypeAnswer -> userTypeAnswer.isValid() }
)
observableList += filteredUserTypes.mapIndexed { index, userTypeOption ->
when (userTypeOption) {
UserTypeAnswer.LEARNER -> MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(R.string.user_type_answer_learner),
index,
this
)
UserTypeAnswer.TEACHER -> MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(R.string.user_type_answer_teacher),
index,
this
)
UserTypeAnswer.PARENT -> MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(R.string.user_type_answer_parent),
index,
this
)
UserTypeAnswer.OTHER -> MultipleChoiceOptionContentViewModel(
resourceHandler.getStringInLocale(R.string.user_type_answer_other),
index,
this
)
else -> throw IllegalStateException("Invalid UserTypeAnswer")
}
}
return observableList
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import org.oppia.android.util.enumfilter.filterByEnumCondition
import javax.inject.Inject

/** The presenter for [TopicLessonsFragment]. */
Expand Down Expand Up @@ -161,18 +162,18 @@ class TopicLessonsFragmentPresenter @Inject constructor(

val chapterSummaries = storySummaryViewModel
.storySummary.chapterList
val completedChapterCount =
chapterSummaries.map(ChapterSummary::getChapterPlayState)
.filter {
it == ChapterPlayState.COMPLETED
}
.size
val completedChapterCount = filterByEnumCondition(
chapterSummaries.map(ChapterSummary::getChapterPlayState),
{ it },
{ it == ChapterPlayState.COMPLETED }
).size

val inProgressChapterCount =
chapterSummaries.map(ChapterSummary::getChapterPlayState)
.filter {
it == ChapterPlayState.IN_PROGRESS_SAVED
}
.size
filterByEnumCondition(
chapterSummaries.map(ChapterSummary::getChapterPlayState),
{ it },
{ it == ChapterPlayState.IN_PROGRESS_SAVED }
).size

val storyPercentage: Int =
(completedChapterCount * 100) / storySummaryViewModel.storySummary.chapterCount
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ kt_android_library(
"//domain/src/main/java/org/oppia/android/domain/oppialogger:prod_module",
"//model/src/main/proto:performance_metrics_event_logger_java_proto_lite",
"//utility/src/main/java/org/oppia/android/util/data:data_provider",
"//utility/src/main/java/org/oppia/android/util/enumfilter:enum_filter_util",
"//utility/src/main/java/org/oppia/android/util/logging:console_logger",
"//utility/src/main/java/org/oppia/android/util/logging:exception_logger",
"//utility/src/main/java/org/oppia/android/util/logging/performancemetrics:performance_metrics_assessor",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.oppia.android.app.model.ScreenName
import org.oppia.android.data.persistence.PersistentCacheStore
import org.oppia.android.domain.oppialogger.PerformanceMetricsLogStorageCacheSize
import org.oppia.android.util.data.DataProvider
import org.oppia.android.util.enumfilter.filterByEnumCondition
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.ExceptionLogger
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsAssessor
Expand Down Expand Up @@ -128,9 +129,11 @@ class PerformanceMetricsController @Inject constructor(
* priority is returned.
*/
private fun getLeastRecentMetricLogIndex(oppiaMetricLogs: OppiaMetricLogs): Int? =
oppiaMetricLogs.oppiaMetricLogList.withIndex()
.filter { it.value.priority == Priority.LOW_PRIORITY }
.minByOrNull { it.value.timestampMillis }?.index
filterByEnumCondition(
oppiaMetricLogs.oppiaMetricLogList.withIndex().toList(),
{ it.value.priority },
{ it == Priority.LOW_PRIORITY }
).minByOrNull { it.value.timestampMillis }?.index
?: getLeastRecentMediumPriorityEventIndex(oppiaMetricLogs)

/**
Expand All @@ -142,9 +145,11 @@ class PerformanceMetricsController @Inject constructor(
* priority is returned.
*/
private fun getLeastRecentMediumPriorityEventIndex(oppiaMetricLogs: OppiaMetricLogs): Int? =
oppiaMetricLogs.oppiaMetricLogList.withIndex()
.filter { it.value.priority == Priority.MEDIUM_PRIORITY }
.minByOrNull { it.value.timestampMillis }?.index
filterByEnumCondition(
oppiaMetricLogs.oppiaMetricLogList.withIndex().toList(),
{ it.value.priority },
{ it == Priority.MEDIUM_PRIORITY }
).minByOrNull { it.value.timestampMillis }?.index
?: getLeastRecentGeneralEventIndex(oppiaMetricLogs)

/** Returns the index of the least recent event regardless of their priority. */
Expand Down
4 changes: 4 additions & 0 deletions scripts/assets/test_file_exemptions.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -4310,6 +4310,10 @@ test_file_exemption {
exempted_file_path: "utility/src/main/java/org/oppia/android/util/extensions/ContextExtensions.kt"
test_file_not_required: true
}
test_file_exemption {
exempted_file_path: "utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt"
test_file_not_required: true
}
test_file_exemption {
exempted_file_path: "utility/src/main/java/org/oppia/android/util/extensions/StringExtensions.kt"
source_file_is_incompatible_with_code_coverage: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
General purpose utility for filtering enums.
"""

load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")

kt_android_library(
name = "enum_filter_util",
srcs = [
"EnumFilterUtil.kt",
],
visibility = ["//:oppia_api_visibility"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.oppia.android.util.enumfilter

/**
* Filters a collection based on a condition applied to an enum property of each element.
*
* @param E the type of enum values.
* @param T the type of elements in the collection.
* @param collection the collection of elements to filter.
* @param enumExtractor a function that extracts the enum value from each element.
* @param condition a predicate function that determines if an enum value should be included in the result.
* @return a list of elements from the collection that satisfy the condition when their enum property is evaluated.
*/

inline fun <E : Enum<E>, T> filterByEnumCondition(
collection: Collection<T>,
enumExtractor: (T) -> E,
condition: (E) -> Boolean
): List<T> {
return collection.filter { condition(enumExtractor(it)) }
}

0 comments on commit 33d2b8f

Please sign in to comment.