From aab6fd2bfe2b4572339e0d6ac405ec459dcdc9b4 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 12:32:43 +0100 Subject: [PATCH 01/10] Inject AppRepository to let methods in FahrplanMisc become testable. --- .../congress/autoupdate/UpdateService.java | 3 ++- .../congress/changes/ChangeListFragment.java | 8 +++++--- .../favorites/StarredListFragment.java | 5 +++-- .../congress/repositories/AppRepository.kt | 2 +- .../congress/schedule/FahrplanFragment.java | 3 ++- .../congress/schedule/MainActivity.java | 2 +- .../fahrplan/congress/utils/FahrplanMisc.java | 20 +++++++++---------- 7 files changed, 23 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/autoupdate/UpdateService.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/autoupdate/UpdateService.java index ade632a00..e273024dc 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/autoupdate/UpdateService.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/autoupdate/UpdateService.java @@ -38,7 +38,8 @@ public void onParseDone(Boolean result, String version) { MyApp.LogDebug(LOG_TAG, "parseDone: " + result + " , numDays=" + MyApp.meta.getNumDays()); MyApp.task_running = TASKS.NONE; MyApp.fahrplan_xml = null; - List changesList = FahrplanMisc.readChanges(this); + AppRepository appRepository = AppRepository.Companion.getInstance(this); + List changesList = FahrplanMisc.readChanges(appRepository); if (!changesList.isEmpty()) { showScheduleUpdateNotification(version, changesList.size()); } diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/changes/ChangeListFragment.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/changes/ChangeListFragment.java index 8420073b2..25e06876a 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/changes/ChangeListFragment.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/changes/ChangeListFragment.java @@ -77,8 +77,9 @@ public void onCreate(Bundle savedInstanceState) { } FragmentActivity activity = getActivity(); - changesList = FahrplanMisc.readChanges(activity); - Meta meta = AppRepository.Companion.getInstance(activity).readMeta(); + AppRepository appRepository = AppRepository.Companion.getInstance(activity); + changesList = FahrplanMisc.readChanges(appRepository); + Meta meta = appRepository.readMeta(); mAdapter = new LectureChangesArrayAdapter(activity, changesList, meta.getNumDays()); MyApp.LogDebug(LOG_TAG, "onCreate, " + changesList.size() + " changes"); } @@ -128,7 +129,8 @@ public void onDetach() { } public void onRefresh() { - List updatedChanges = FahrplanMisc.readChanges(getActivity()); + AppRepository appRepository = AppRepository.Companion.getInstance(getActivity()); + List updatedChanges = FahrplanMisc.readChanges(appRepository); if (changesList != null) { changesList.clear(); changesList.addAll(updatedChanges); diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/favorites/StarredListFragment.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/favorites/StarredListFragment.java index ca95bf7b0..8e1a6c715 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/favorites/StarredListFragment.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/favorites/StarredListFragment.java @@ -128,8 +128,8 @@ public void onResume() { private void initStarredList() { FragmentActivity activity = getActivity(); - starredList = FahrplanMisc.getStarredLectures(activity); AppRepository appRepository = AppRepository.Companion.getInstance(activity); + starredList = FahrplanMisc.getStarredLectures(appRepository); Meta meta = appRepository.readMeta(); mAdapter = new LectureArrayAdapter(activity, starredList, meta.getNumDays()); MyApp.LogDebug(LOG_TAG, "initStarredList: " + starredList.size() + " favorites"); @@ -174,7 +174,8 @@ public void onDetach() { } public void onRefresh(@NonNull Context context) { - List starred = FahrplanMisc.getStarredLectures(context); + AppRepository appRepository = AppRepository.Companion.getInstance(context); + List starred = FahrplanMisc.getStarredLectures(appRepository); if (starredList != null) { starredList.clear(); starredList.addAll(starred); diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt index 311c7f908..51827f6db 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/repositories/AppRepository.kt @@ -75,7 +75,7 @@ class AppRepository private constructor(val context: Context) { // Parsing scheduleNetworkRepository.parseSchedule(fetchScheduleResult.scheduleXml, fetchScheduleResult.eTag, onUpdateLectures = { lectures -> - val oldLectures = FahrplanMisc.loadLecturesForAllDays(context) + val oldLectures = FahrplanMisc.loadLecturesForAllDays(this) val newLectures = lectures.toLecturesAppModel2() val hasChanged = ScheduleChanges.hasScheduleChanged(newLectures, oldLectures) if (hasChanged) { diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java index cfab0102c..ba5823a16 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java @@ -698,7 +698,8 @@ public static void loadLectureList(Context context, int day, boolean force) { return; } - MyApp.lectureList = FahrplanMisc.loadLecturesForDayIndex(context, day); + AppRepository appRepository = AppRepository.Companion.getInstance(context); + MyApp.lectureList = FahrplanMisc.loadLecturesForDayIndex(appRepository, day); if (MyApp.lectureList == null) { return; } diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/MainActivity.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/MainActivity.java index 391f9ae54..6a8b3fa7c 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/MainActivity.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/MainActivity.java @@ -286,8 +286,8 @@ void showChangesDialog() { Fragment fragment = findFragment(ChangesDialog.FRAGMENT_TAG); if (fragment == null) { requiresScheduleReload = true; - List changedLectures = FahrplanMisc.readChanges(this); AppRepository appRepository = AppRepository.Companion.getInstance(this); + List changedLectures = FahrplanMisc.readChanges(appRepository); Meta meta = appRepository.readMeta(); String scheduleVersion = meta.getVersion(); DialogFragment changesDialog = ChangesDialog.newInstance( diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 8f351c9e4..1e3f452d3 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -165,21 +165,19 @@ public void onRescheduleInitialAlarm(long interval, long nextFetch) { } @NonNull - public static List loadLecturesForAllDays(@NonNull Context context) { - return loadLecturesForDayIndex(context, ALL_DAYS); + public static List loadLecturesForAllDays(@NonNull AppRepository appRepository) { + return loadLecturesForDayIndex(appRepository, ALL_DAYS); } /** * Load all Lectures from the DB on the day specified * - * @param context The Android Context - * @param day The day to load lectures for (0..), or -1 for all days + * @param appRepository The application repository to retrieve lectures from. + * @param day The day to load lectures for (0..), or -1 for all days * @return ArrayList of Lecture objects */ @NonNull - public static List loadLecturesForDayIndex(@NonNull Context context, int day) { - AppRepository appRepository = AppRepository.Companion.getInstance(context); - + public static List loadLecturesForDayIndex(@NonNull AppRepository appRepository, int day) { List lectures; if (day == ALL_DAYS) { MyApp.LogDebug(LOG_TAG, "Loading lectures for all days."); @@ -244,9 +242,9 @@ public static int getCancelledLectureCount(@NonNull List list, boolean } @NonNull - public static List readChanges(Context context) { + public static List readChanges(@NonNull AppRepository appRepository) { MyApp.LogDebug(LOG_TAG, "readChanges"); - List changesList = loadLecturesForAllDays(context); + List changesList = loadLecturesForAllDays(appRepository); if (changesList.isEmpty()) { return changesList; } @@ -263,8 +261,8 @@ public static List readChanges(Context context) { } @NonNull - public static List getStarredLectures(@NonNull Context context) { - List starredList = loadLecturesForAllDays(context); + public static List getStarredLectures(@NonNull AppRepository appRepository) { + List starredList = loadLecturesForAllDays(appRepository); if (starredList.isEmpty()) { return starredList; } From 11a140da44d98e644a149acb23ee1274ec868916 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 12:39:20 +0100 Subject: [PATCH 02/10] Resolve Lint warning indicating that null check is not needed. --- .../tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java index ba5823a16..92be2dcc2 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java @@ -700,7 +700,7 @@ public static void loadLectureList(Context context, int day, boolean force) { AppRepository appRepository = AppRepository.Companion.getInstance(context); MyApp.lectureList = FahrplanMisc.loadLecturesForDayIndex(appRepository, day); - if (MyApp.lectureList == null) { + if (MyApp.lectureList.isEmpty()) { return; } From 98b842bd6d7b5659fdb0f2557663051898385a37 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 3 Jan 2019 21:36:52 +0100 Subject: [PATCH 03/10] Extract lecture filtering to let it become testable. --- .../congress/schedule/FahrplanFragment.java | 11 +---------- .../fahrplan/congress/utils/FahrplanMisc.java | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java index 92be2dcc2..dc2c0f380 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/schedule/FahrplanFragment.java @@ -699,19 +699,10 @@ public static void loadLectureList(Context context, int day, boolean force) { } AppRepository appRepository = AppRepository.Companion.getInstance(context); - MyApp.lectureList = FahrplanMisc.loadLecturesForDayIndex(appRepository, day); + MyApp.lectureList = FahrplanMisc.getUncanceledLectures(appRepository, day); if (MyApp.lectureList.isEmpty()) { return; } - - int lectureIndex = MyApp.lectureList.size() - 1; - while (lectureIndex >= 0) { - Lecture l = MyApp.lectureList.get(lectureIndex); - if (l.changedIsCanceled) { - MyApp.lectureList.remove(lectureIndex); - } - lectureIndex--; - } MyApp.lectureListDay = day; MyApp.roomsMap.clear(); diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 1e3f452d3..321215f43 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -280,4 +280,19 @@ public static List getStarredLectures(@NonNull AppRepository appReposit MyApp.LogDebug(LOG_TAG, starredList.size() + " lectures starred."); return starredList; } + + @NonNull + public static List getUncanceledLectures(@NonNull AppRepository appRepository, int dayIndex) { + List lectures = FahrplanMisc.loadLecturesForDayIndex(appRepository, dayIndex); + int lectureIndex = lectures.size() - 1; + while (lectureIndex >= 0) { + Lecture l = lectures.get(lectureIndex); + if (l.changedIsCanceled) { + lectures.remove(lectureIndex); + } + lectureIndex--; + } + return lectures; + } + } From fbb5417bc7549f56f183bf2be3ceca4080baad81 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 12:34:37 +0100 Subject: [PATCH 04/10] Unit test methods to filter lectures. --- app/build.gradle | 1 + .../fahrplan/congress/utils/FahrplanMisc.java | 4 +- .../congress/utils/FahrplanMiscTest.kt | 136 ++++++++++++++++++ .../org.mockito.plugins.MockMaker | 1 + .../fahrplan/congress/Dependencies.kt | 2 + 5 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt create mode 100644 app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/app/build.gradle b/app/build.gradle index 9df119421..4301168d0 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -184,6 +184,7 @@ dependencies { testImplementation Libs.assertjAndroid testImplementation Libs.supportLibraryAnnotations testImplementation Libs.mockitoCore + testImplementation Libs.mockitoKotlin testImplementation Libs.threeTenBp } diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 321215f43..25d3e1f45 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -5,6 +5,7 @@ import android.content.Context; import android.content.Intent; import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; import android.text.format.Time; import org.ligi.tracedroid.logging.Log; @@ -35,7 +36,8 @@ public class FahrplanMisc { private static final String LOG_TAG = "FahrplanMisc"; - private static final int ALL_DAYS = -1; + @VisibleForTesting + public static final int ALL_DAYS = -1; private static final DateFormat TIME_TEXT_DATE_FORMAT = SimpleDateFormat.getDateTimeInstance(SimpleDateFormat.SHORT, SimpleDateFormat.SHORT); diff --git a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt new file mode 100644 index 000000000..eba051a2a --- /dev/null +++ b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt @@ -0,0 +1,136 @@ +package nerd.tuxmobil.fahrplan.congress.utils + +import com.nhaarman.mockitokotlin2.doReturn +import com.nhaarman.mockitokotlin2.mock +import nerd.tuxmobil.fahrplan.congress.repositories.AppRepository +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import nerd.tuxmobil.fahrplan.congress.models.Lecture as Event + +class FahrplanMiscTest { + + companion object { + + private val EVENT_1001 = Event("1001").apply { + highlight = false + changedIsCanceled = false + } + + private val EVENT_1002 = Event("1002").apply { + highlight = true + changedIsCanceled = false + } + + private val EVENT_1003 = Event("1003").apply { + highlight = true + changedIsCanceled = true + } + + private val EVENT_1004 = Event("1004").apply { + highlight = false + changedIsCanceled = true + } + + private val EVENT_2001 = Event("2001").apply { + changedIsCanceled = false + changedTitle = false + changedIsNew = false + } + + private val EVENT_2002 = Event("2002").apply { + changedIsCanceled = true + changedTitle = false + changedIsNew = false + } + + private val EVENT_2003 = Event("2003").apply { + changedIsCanceled = false + changedTitle = true + changedIsNew = false + } + + private val EVENT_2004 = Event("2004").apply { + changedIsCanceled = false + changedTitle = false + changedIsNew = true + } + + private val EVENT_2005 = Event("2005").apply { + changedIsCanceled = true + changedTitle = true + changedIsNew = true + } + + private val EVENT_3001 = Event("3001").apply { + changedIsCanceled = false + } + + private val EVENT_3002 = Event("3002").apply { + changedIsCanceled = true + } + + } + + @Test + fun getStarredLecturesWithEmptyList() { + val appRepository = mock { + on { readLecturesOrderedByDateUtc() } doReturn emptyList() + } + assertThat(FahrplanMisc.getStarredLectures(appRepository)).isEmpty() + } + + @Test + fun getStarredLecturesWithAllEvents() { + val appRepository = mock { + val events = mutableListOf(EVENT_1001, EVENT_1002, EVENT_1003, EVENT_1004) + on { readLecturesOrderedByDateUtc() } doReturn events + } + val starredEvents = FahrplanMisc.getStarredLectures(appRepository) + assertThat(starredEvents).isNotEmpty() + assertThat(starredEvents.size).isEqualTo(1) + assertThat(starredEvents).contains(EVENT_1002) + assertThat(starredEvents).doesNotContain(EVENT_1001, EVENT_1003, EVENT_1004) + } + + @Test + fun readChangesWithEmptyList() { + val appRepository = mock { + on { readLecturesOrderedByDateUtc() } doReturn emptyList() + } + assertThat(FahrplanMisc.readChanges(appRepository)).isEmpty() + } + + @Test + fun readChangesWithAllEvents() { + val appRepository = mock { + val events = mutableListOf(EVENT_2001, EVENT_2002, EVENT_2003, EVENT_2004, EVENT_2005) + on { readLecturesOrderedByDateUtc() } doReturn events + } + val changedEvents = FahrplanMisc.readChanges(appRepository) + assertThat(changedEvents).isNotEmpty() + assertThat(changedEvents.size).isEqualTo(4) + assertThat(changedEvents).contains(EVENT_2002, EVENT_2003, EVENT_2004, EVENT_2005) + assertThat(changedEvents).doesNotContain(EVENT_2001) + } + + @Test + fun getUpcomingLecturesWithEmptyList() { + val appRepository = mock { + on { readLecturesOrderedByDateUtc() } doReturn emptyList() + } + assertThat(FahrplanMisc.getUncanceledLectures(appRepository, FahrplanMisc.ALL_DAYS)).isEmpty() + } + + @Test + fun getUpcomingLecturesWithAllEvents() { + val appRepository = mock { + on { readLecturesOrderedByDateUtc() } doReturn mutableListOf(EVENT_3001, EVENT_3002) + } + val upcomingEvents = FahrplanMisc.getUncanceledLectures(appRepository, FahrplanMisc.ALL_DAYS) + assertThat(upcomingEvents).isNotEmpty() + assertThat(upcomingEvents.size).isEqualTo(1) + assertThat(upcomingEvents).contains(EVENT_3001) + assertThat(upcomingEvents).doesNotContain(EVENT_3002) + } + +} diff --git a/app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 000000000..1f0955d45 --- /dev/null +++ b/app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline diff --git a/buildSrc/src/main/kotlin/nerd/tuxmobil/fahrplan/congress/Dependencies.kt b/buildSrc/src/main/kotlin/nerd/tuxmobil/fahrplan/congress/Dependencies.kt index ea6d3208c..9664a1bf5 100644 --- a/buildSrc/src/main/kotlin/nerd/tuxmobil/fahrplan/congress/Dependencies.kt +++ b/buildSrc/src/main/kotlin/nerd/tuxmobil/fahrplan/congress/Dependencies.kt @@ -11,6 +11,7 @@ object Versions { val junit = "4.12" val kotlin = "1.2.51" val mockito = "2.23.0" + val mockitoKotlin = "2.1.0" val okhttp = "3.12.0" val snackengage = "0.18" val sonarQubeGradle = "2.6.2" @@ -41,6 +42,7 @@ object Libs { val junit = "junit:junit:${Versions.junit}" val kotlinStdlib = "org.jetbrains.kotlin:kotlin-stdlib:${Versions.kotlin}" val mockitoCore = "org.mockito:mockito-core:${Versions.mockito}" + val mockitoKotlin = "com.nhaarman.mockitokotlin2:mockito-kotlin:${Versions.mockitoKotlin}" val okhttp = "com.squareup.okhttp3:okhttp:${Versions.okhttp}" val okhttpLoggingInterceptor = "com.squareup.okhttp3:logging-interceptor:${Versions.okhttp}" val snackengagePlayrate = "com.github.ligi.snackengage:snackengage-playrate:${Versions.snackengage}" From 5b0f3d0ff1986a9a217de5a78e22697e63b23a4b Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 3 Jan 2019 21:36:52 +0100 Subject: [PATCH 05/10] Simplify list filtering by using CollectionsKt.filterNot(). --- .../fahrplan/congress/utils/FahrplanMisc.java | 33 +++---------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 25d3e1f45..0244def97 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -32,6 +32,8 @@ import nerd.tuxmobil.fahrplan.congress.models.SchedulableAlarm; import nerd.tuxmobil.fahrplan.congress.repositories.AppRepository; +import static kotlin.collections.CollectionsKt.filterNot; + public class FahrplanMisc { @@ -250,14 +252,7 @@ public static List readChanges(@NonNull AppRepository appRepository) { if (changesList.isEmpty()) { return changesList; } - int lectureIndex = changesList.size() - 1; - while (lectureIndex >= 0) { - Lecture l = changesList.get(lectureIndex); - if (!l.isChanged() && !l.changedIsCanceled && !l.changedIsNew) { - changesList.remove(l); - } - lectureIndex--; - } + changesList = filterNot(changesList, event -> !event.isChanged() && !event.changedIsCanceled && !event.changedIsNew); MyApp.LogDebug(LOG_TAG, changesList.size() + " lectures changed."); return changesList; } @@ -268,17 +263,7 @@ public static List getStarredLectures(@NonNull AppRepository appReposit if (starredList.isEmpty()) { return starredList; } - int lectureIndex = starredList.size() - 1; - while (lectureIndex >= 0) { - Lecture l = starredList.get(lectureIndex); - if (!l.highlight) { - starredList.remove(l); - } - if (l.changedIsCanceled) { - starredList.remove(l); - } - lectureIndex--; - } + starredList = filterNot(starredList, event -> !event.highlight || event.changedIsCanceled); MyApp.LogDebug(LOG_TAG, starredList.size() + " lectures starred."); return starredList; } @@ -286,15 +271,7 @@ public static List getStarredLectures(@NonNull AppRepository appReposit @NonNull public static List getUncanceledLectures(@NonNull AppRepository appRepository, int dayIndex) { List lectures = FahrplanMisc.loadLecturesForDayIndex(appRepository, dayIndex); - int lectureIndex = lectures.size() - 1; - while (lectureIndex >= 0) { - Lecture l = lectures.get(lectureIndex); - if (l.changedIsCanceled) { - lectures.remove(lectureIndex); - } - lectureIndex--; - } - return lectures; + return filterNot(lectures, event -> event.changedIsCanceled); } } From 3f9612f4d476969f05822bea05564057dc7babe9 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 13:43:52 +0100 Subject: [PATCH 06/10] Unit test methods to count lectures. --- .../congress/utils/FahrplanMiscTest.kt | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt index eba051a2a..58d84b65e 100644 --- a/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt +++ b/app/src/test/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMiscTest.kt @@ -69,6 +69,66 @@ class FahrplanMiscTest { changedIsCanceled = true } + private val EVENT_4001 = Event("4001").apply { + highlight = false + changedTitle = false + } + + private val EVENT_4002 = Event("4002").apply { + highlight = true + changedTitle = false + } + + private val EVENT_4003 = Event("4003").apply { + highlight = true + changedTitle = true + } + + private val EVENT_4004 = Event("4004").apply { + highlight = false + changedTitle = true + } + + private val EVENT_5001 = Event("5001").apply { + highlight = false + changedIsNew = false + } + + private val EVENT_5002 = Event("5002").apply { + highlight = true + changedIsNew = false + } + + private val EVENT_5003 = Event("5003").apply { + highlight = true + changedIsNew = true + } + + private val EVENT_5004 = Event("5004").apply { + highlight = false + changedIsNew = true + } + + private val EVENT_6001 = Event("6001").apply { + highlight = false + changedIsCanceled = false + } + + private val EVENT_6002 = Event("6002").apply { + highlight = true + changedIsCanceled = false + } + + private val EVENT_6003 = Event("6003").apply { + highlight = true + changedIsCanceled = true + } + + private val EVENT_6004 = Event("6004").apply { + highlight = false + changedIsCanceled = true + } + } @Test @@ -133,4 +193,70 @@ class FahrplanMiscTest { assertThat(upcomingEvents).doesNotContain(EVENT_3002) } + @Test + fun getChangedLectureCountWithEmptyListWithFavsOnly() { + assertThat(FahrplanMisc.getChangedLectureCount(emptyList(), true)).isEqualTo(0) + } + + @Test + fun getChangedLectureCountWithEmptyList() { + assertThat(FahrplanMisc.getChangedLectureCount(emptyList(), false)).isEqualTo(0) + } + + @Test + fun getChangedLectureCountWithAllEventsWithFavsOnly() { + val events = listOf(EVENT_4001, EVENT_4002, EVENT_4003, EVENT_4004) + assertThat(FahrplanMisc.getChangedLectureCount(events, true)).isEqualTo(1) + } + + @Test + fun getChangedLectureCountWithAllEvents() { + val events = listOf(EVENT_4001, EVENT_4002, EVENT_4003, EVENT_4004) + assertThat(FahrplanMisc.getChangedLectureCount(events, false)).isEqualTo(2) + } + + @Test + fun getNewLectureCountWithEmptyListWithFavsOnly() { + assertThat(FahrplanMisc.getNewLectureCount(emptyList(), true)).isEqualTo(0) + } + + @Test + fun getNewLectureCountWithEmptyList() { + assertThat(FahrplanMisc.getNewLectureCount(emptyList(), false)).isEqualTo(0) + } + + @Test + fun getNewLectureCountWithAllEventsWithFavsOnly() { + val events = listOf(EVENT_5001, EVENT_5002, EVENT_5003, EVENT_5004) + assertThat(FahrplanMisc.getNewLectureCount(events, true)).isEqualTo(1) + } + + @Test + fun getNewLectureCountWithAllEvents() { + val events = listOf(EVENT_5001, EVENT_5002, EVENT_5003, EVENT_5004) + assertThat(FahrplanMisc.getNewLectureCount(events, false)).isEqualTo(2) + } + + @Test + fun getCancelledLectureCountWithEmptyListWithFavsOnly() { + assertThat(FahrplanMisc.getCancelledLectureCount(emptyList(), true)).isEqualTo(0) + } + + @Test + fun getCancelledLectureCountWithEmptyList() { + assertThat(FahrplanMisc.getCancelledLectureCount(emptyList(), false)).isEqualTo(0) + } + + @Test + fun getCancelledLectureCountWithAllEventsWithFavsOnly() { + val events = listOf(EVENT_6001, EVENT_6002, EVENT_6003, EVENT_6004) + assertThat(FahrplanMisc.getCancelledLectureCount(events, true)).isEqualTo(1) + } + + @Test + fun getCancelledLectureCountWithAllEvents() { + val events = listOf(EVENT_6001, EVENT_6002, EVENT_6003, EVENT_6004) + assertThat(FahrplanMisc.getCancelledLectureCount(events, false)).isEqualTo(2) + } + } From d06bb88b84767d6cc7b02096ad2c475694103e32 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 13:47:02 +0100 Subject: [PATCH 07/10] Simplify elements counting by using CollectionsKt.count(). --- .../fahrplan/congress/utils/FahrplanMisc.java | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 0244def97..01ec75328 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -32,6 +32,7 @@ import nerd.tuxmobil.fahrplan.congress.models.SchedulableAlarm; import nerd.tuxmobil.fahrplan.congress.repositories.AppRepository; +import static kotlin.collections.CollectionsKt.count; import static kotlin.collections.CollectionsKt.filterNot; @@ -205,42 +206,19 @@ public static List loadLecturesForDayIndex(@NonNull AppRepository appRe } public static int getChangedLectureCount(@NonNull List list, boolean favsOnly) { - int count = 0; - if (list.isEmpty()) { - return 0; - } - for (int lectureIndex = 0; lectureIndex < list.size(); lectureIndex++) { - Lecture l = list.get(lectureIndex); - if (l.isChanged() && (!favsOnly || l.highlight)) { - count++; - } - } + int count = count(list, event -> event.isChanged() && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getChangedLectureCount " + favsOnly + ":" + count); return count; } public static int getNewLectureCount(@NonNull List list, boolean favsOnly) { - int count = 0; - if (list.isEmpty()) { - return 0; - } - for (int lectureIndex = 0; lectureIndex < list.size(); lectureIndex++) { - Lecture l = list.get(lectureIndex); - if (l.changedIsNew && (!favsOnly || l.highlight)) count++; - } + int count = count(list, event -> event.changedIsNew && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getNewLectureCount " + favsOnly + ":" + count); return count; } public static int getCancelledLectureCount(@NonNull List list, boolean favsOnly) { - int count = 0; - if (list.isEmpty()) { - return 0; - } - for (int lectureIndex = 0; lectureIndex < list.size(); lectureIndex++) { - Lecture l = list.get(lectureIndex); - if (l.changedIsCanceled && (!favsOnly || l.highlight)) count++; - } + int count = count(list, event -> event.changedIsCanceled && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getCancelledLectureCount " + favsOnly + ":" + count); return count; } From 3bac56ddff0e6a50dfae84a75d5827380d762904 Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 15:49:35 +0100 Subject: [PATCH 08/10] Communicate that the list of lectures is not modified by these methods. --- .../nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 01ec75328..3355b4b37 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -205,19 +205,19 @@ public static List loadLecturesForDayIndex(@NonNull AppRepository appRe return lectures; } - public static int getChangedLectureCount(@NonNull List list, boolean favsOnly) { + public static int getChangedLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.isChanged() && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getChangedLectureCount " + favsOnly + ":" + count); return count; } - public static int getNewLectureCount(@NonNull List list, boolean favsOnly) { + public static int getNewLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.changedIsNew && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getNewLectureCount " + favsOnly + ":" + count); return count; } - public static int getCancelledLectureCount(@NonNull List list, boolean favsOnly) { + public static int getCancelledLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.changedIsCanceled && (!favsOnly || event.highlight)); MyApp.LogDebug(LOG_TAG, "getCancelledLectureCount " + favsOnly + ":" + count); return count; From e720399b56cc96a36f80ae2f01dcd7c96547f97b Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 16:04:45 +0100 Subject: [PATCH 09/10] DRY debug output. --- .../nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 3355b4b37..7ecd65f48 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -195,7 +195,7 @@ public static List loadLecturesForDayIndex(@NonNull AppRepository appRe List highlights = appRepository.readHighlights(); for (Highlight highlight : highlights) { - MyApp.LogDebug(LOG_TAG, "highlight = " + highlight); + MyApp.LogDebug(LOG_TAG, highlight.toString()); for (Lecture lecture : lectures) { if (lecture.lecture_id.equals("" + highlight.getEventId())) { lecture.highlight = highlight.isHighlight(); From 9329c3642952ae61f14d73cbfb4ec79cdc9caafa Mon Sep 17 00:00:00 2001 From: Tobias Preuss Date: Thu, 10 Jan 2019 16:34:47 +0100 Subject: [PATCH 10/10] Improve readability of debug output. --- .../nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java index 7ecd65f48..4ac1d1d09 100644 --- a/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java +++ b/app/src/main/java/nerd/tuxmobil/fahrplan/congress/utils/FahrplanMisc.java @@ -207,19 +207,19 @@ public static List loadLecturesForDayIndex(@NonNull AppRepository appRe public static int getChangedLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.isChanged() && (!favsOnly || event.highlight)); - MyApp.LogDebug(LOG_TAG, "getChangedLectureCount " + favsOnly + ":" + count); + MyApp.LogDebug(LOG_TAG, count + " changed lectures, favsOnly = " + favsOnly); return count; } public static int getNewLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.changedIsNew && (!favsOnly || event.highlight)); - MyApp.LogDebug(LOG_TAG, "getNewLectureCount " + favsOnly + ":" + count); + MyApp.LogDebug(LOG_TAG, count + " new lectures, favsOnly = " + favsOnly); return count; } public static int getCancelledLectureCount(@NonNull final List list, boolean favsOnly) { int count = count(list, event -> event.changedIsCanceled && (!favsOnly || event.highlight)); - MyApp.LogDebug(LOG_TAG, "getCancelledLectureCount " + favsOnly + ":" + count); + MyApp.LogDebug(LOG_TAG, count + " canceled lectures, favsOnly = " + favsOnly); return count; }