fix(map_extension): revert Map copy in fillEmptyKeyValues (#121)

* fix(map_extension): revert Map copy in `fillEmptyKeyValues` * test(map_extension): adapt tests for mutation
albertms10 · Dec 4, 2022 · c015429 · c015429
1 parent d7cca76
commit c015429
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 58 deletions.
diff --git a/lib/utils/map_extension.dart b/lib/utils/map_extension.dart
@@ -1,29 +1,25 @@
 /// Map extension.
 extension MapExtension<K, V> on Map<K, V> {
-  /// Returns a new [Map] filled with the result of [ifAbsent] for [keys] that
-  /// are not present in this [Map].
+  /// Fills this [Map] with the result of [ifAbsent] for [keys] that are not
+  /// present.
   ///
-  /// Unlike [Map.update], this method does not mutate this [Map].
+  /// Like [Map.update], this method mutates this [Map].
   ///
   /// Example:
   /// ```dart
-  /// const map = <int, bool?>{1: true, 2: false};
-  /// final filled = map.fillEmptyKeyValues(
-  ///   keys: const [0, 1, 2, 5],
-  ///   ifAbsent: () => null,
-  /// );
-  /// assert(filled == const {0: null, 1: true, 2: false, 5: null});
+  /// final map = <int, bool?>{1: true, 2: false}
+  ///   ..fillEmptyKeyValues(
+  ///     keys: const [0, 1, 2, 5],
+  ///     ifAbsent: () => null,
+  ///   );
+  /// assert(map == const {0: null, 1: true, 2: false, 5: null});
   /// ```
-  Map<K, V> fillEmptyKeyValues({
+  void fillEmptyKeyValues({
     required Iterable<K> keys,
     required V Function() ifAbsent,
   }) {
-    final map = Map<K, V>.of(this);
-
     for (final key in keys) {
-      map.update(key, (value) => value, ifAbsent: ifAbsent);
+      update(key, (value) => value, ifAbsent: ifAbsent);
     }
-
-    return map;
   }
 }
diff --git a/lib/widgets/cabin/cabins_table.dart b/lib/widgets/cabin/cabins_table.dart
@@ -32,18 +32,16 @@ class CabinsTable extends StatelessWidget {
                 recurringBookingsCount:
                     cabin.generatedBookingsFromRecurring.length,
                 occupiedDuration: cabin.occupiedDuration(),
-                occupiedDurationPerWeek: cabin
-                    .occupiedDurationPerWeek(
-                      DateRange(startDate: lastYear, endDate: now),
-                    )
-                    .fillEmptyKeyValues(
-                      keys: DateRanger.rangeDateTimeList(
-                        lastYear.firstDayOfWeek,
-                        now.firstDayOfWeek,
-                        interval: const Duration(days: DateTime.daysPerWeek),
-                      ),
-                      ifAbsent: () => Duration.zero,
+                occupiedDurationPerWeek: cabin.occupiedDurationPerWeek(
+                  DateRange(startDate: lastYear, endDate: now),
+                )..fillEmptyKeyValues(
+                    keys: DateRanger.rangeDateTimeList(
+                      lastYear.firstDayOfWeek,
+                      now.firstDayOfWeek,
+                      interval: const Duration(days: DateTime.daysPerWeek),
                     ),
+                    ifAbsent: () => Duration.zero,
+                  ),
                 mostOccupiedTimeRanges: cabin
                     .mostOccupiedTimeRange()
                     .compactConsecutive(

diff --git a/lib/widgets/pages/summary_page.dart b/lib/widgets/pages/summary_page.dart
@@ -217,13 +217,13 @@ class _PopularTimesStatistics extends StatelessWidget {
       icon: Icons.watch_later,
       items: [
         PopularTimesBarChart(
-          timeRangesOccupancy:
-              cabinManager.accumulatedTimeRangesOccupancy().fillEmptyKeyValues(
-            keys: [
-              for (var i = 9; i < 22; i++) TimeOfDay(hour: i, minute: 0),
-            ],
-            ifAbsent: () => Duration.zero,
-          ),
+          timeRangesOccupancy: cabinManager.accumulatedTimeRangesOccupancy()
+            ..fillEmptyKeyValues(
+              keys: [
+                for (var i = 9; i < 22; i++) TimeOfDay(hour: i, minute: 0),
+              ],
+              ifAbsent: () => Duration.zero,
+            ),
         ),
       ],
     );

diff --git a/lib/widgets/school_year/school_years_table.dart b/lib/widgets/school_year/school_years_table.dart
@@ -30,14 +30,14 @@ class SchoolYearsTable extends StatelessWidget {
                 occupiedDuration: cabinManager.totalOccupiedDuration(
                   dateRange: schoolYear,
                 ),
-                occupiedDurationPerWeek: cabinManager
-                    .occupiedDurationPerWeek(schoolYear)
-                    .fillEmptyKeyValues(
-                      keys: schoolYear.dateTimeList(
-                        interval: const Duration(days: DateTime.daysPerWeek),
+                occupiedDurationPerWeek:
+                    cabinManager.occupiedDurationPerWeek(schoolYear)
+                      ..fillEmptyKeyValues(
+                        keys: schoolYear.dateTimeList(
+                          interval: const Duration(days: DateTime.daysPerWeek),
+                        ),
+                        ifAbsent: () => Duration.zero,
                       ),
-                      ifAbsent: () => Duration.zero,
-                    ),
                 mostOccupiedTimeRanges: cabinManager
                     .mostOccupiedTimeRange(schoolYear)
                     .compactConsecutive(

diff --git a/test/utils/map_extension_test.dart b/test/utils/map_extension_test.dart
@@ -7,28 +7,31 @@ void main() {
     group('.fillEmptyKeyValues', () {
       test('should return the same Map when no keys are provided', () {
         const map = <int, bool?>{1: true, 2: true};
-        final filled = map.fillEmptyKeyValues(
-          keys: const [],
-          ifAbsent: () => null,
-        );
+        final filled = Map.of(map)
+          ..fillEmptyKeyValues(
+            keys: const [],
+            ifAbsent: () => null,
+          );
         expect(filled, map);
       });
 
       test('should return the same Map when all keys are already present', () {
         const map = <int, bool?>{1: true, 2: true};
-        final filled = map.fillEmptyKeyValues(
-          keys: const [1, 2],
-          ifAbsent: () => null,
-        );
+        final filled = Map.of(map)
+          ..fillEmptyKeyValues(
+            keys: const [1, 2],
+            ifAbsent: () => null,
+          );
         expect(filled, map);
       });
 
       test('should return a filled int to bool? Map', () {
         const map = <int, bool?>{2: true, 4: false};
-        final filled = map.fillEmptyKeyValues(
-          keys: const [0, 1, 2, 3, 4, 5],
-          ifAbsent: () => null,
-        );
+        final filled = Map.of(map)
+          ..fillEmptyKeyValues(
+            keys: const [0, 1, 2, 3, 4, 5],
+            ifAbsent: () => null,
+          );
         expect(filled, {
           0: null,
           1: null,
@@ -44,12 +47,13 @@ void main() {
           const TimeOfDay(hour: 9, minute: 0): const Duration(minutes: 30),
           const TimeOfDay(hour: 11, minute: 0): const Duration(minutes: 45),
         };
-        final filled = map.fillEmptyKeyValues(
-          keys: [
-            for (var i = 9; i <= 12; i++) TimeOfDay(hour: i, minute: 0),
-          ],
-          ifAbsent: () => Duration.zero,
-        );
+        final filled = Map.of(map)
+          ..fillEmptyKeyValues(
+            keys: [
+              for (var i = 9; i <= 12; i++) TimeOfDay(hour: i, minute: 0),
+            ],
+            ifAbsent: () => Duration.zero,
+          );
         expect(filled, {
           const TimeOfDay(hour: 9, minute: 0): const Duration(minutes: 30),
           const TimeOfDay(hour: 10, minute: 0): Duration.zero,