From 33d2b8faaaadf0be778f9948261c36fe72fc37b2 Mon Sep 17 00:00:00 2001 From: whyash8 <158801815+whyash8@users.noreply.github.com> Date: Sat, 16 Nov 2024 12:02:05 +0530 Subject: [PATCH] Fix#3146 : Create a generic utility for filtering enums (#5529) ## 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 - [ 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> --- .github/CODEOWNERS | 1 + .../MarketFitItemsViewModel.kt | 9 ++- .../UserTypeItemsViewModel.kt | 69 ++++++++----------- .../lessons/TopicLessonsFragmentPresenter.kt | 23 ++++--- .../domain/oppialogger/analytics/BUILD.bazel | 1 + .../analytics/PerformanceMetricsController.kt | 17 +++-- scripts/assets/test_file_exemptions.textproto | 4 ++ .../oppia/android/util/enumfilter/BUILD.bazel | 13 ++++ .../android/util/enumfilter/EnumFilterUtil.kt | 20 ++++++ 9 files changed, 99 insertions(+), 58 deletions(-) create mode 100644 utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel create mode 100644 utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d2c51eaa7e2..fdf7a2db439 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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 diff --git a/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt b/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt index e2b7fe43a5e..bef9347e3d0 100644 --- a/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt @@ -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. */ @@ -98,8 +99,12 @@ class MarketFitItemsViewModel @Inject constructor( private fun getMarketFitOptions(): ObservableList { val appName = resourceHandler.getStringInLocale(R.string.app_name) val observableList = ObservableArrayList() - 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( diff --git a/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt b/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt index 14482c2775e..a03a78d10c8 100644 --- a/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt @@ -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. */ @@ -97,46 +98,36 @@ class UserTypeItemsViewModel @Inject constructor( private fun getUserTypeOptions(): ObservableArrayList { val observableList = ObservableArrayList() - 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 } diff --git a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt index 66a5a32b2df..1f38909e9d3 100644 --- a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt @@ -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]. */ @@ -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 diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel index f27fbd4b280..b5c7b54bf0b 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel @@ -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", diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt index f518ff6121e..242eccbe1d3 100644 --- a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt +++ b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt @@ -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 @@ -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) /** @@ -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. */ diff --git a/scripts/assets/test_file_exemptions.textproto b/scripts/assets/test_file_exemptions.textproto index 2f4c4422ce9..1ea6be33967 100644 --- a/scripts/assets/test_file_exemptions.textproto +++ b/scripts/assets/test_file_exemptions.textproto @@ -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 diff --git a/utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel b/utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel new file mode 100644 index 00000000000..876c262fe1a --- /dev/null +++ b/utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel @@ -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"], +) diff --git a/utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt b/utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt new file mode 100644 index 00000000000..6c2aae441d6 --- /dev/null +++ b/utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt @@ -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 , T> filterByEnumCondition( + collection: Collection, + enumExtractor: (T) -> E, + condition: (E) -> Boolean +): List { + return collection.filter { condition(enumExtractor(it)) } +}