[compass_app] Standardize on Result factories rather than constructors (#2538)

parlough · web-flow · commit c3c93a82f5af · 2024-12-05T08:38:14.000-05:00
From my review of the recipes PR in flutter/website#11444 (review).
diff --git a/compass_app/app/lib/data/repositories/auth/auth_repository_dev.dart b/compass_app/app/lib/data/repositories/auth/auth_repository_dev.dart
@@ -16,12 +16,12 @@ class AuthRepositoryDev extends AuthRepository {
     required String email,
     required String password,
   }) async {
-    return Result.ok(null);
+    return const Result.ok(null);
   }
 
   /// Logout is always successful in dev scenarios
   @override
   Future<Result<void>> logout() async {
-    return Result.ok(null);
+    return const Result.ok(null);
   }
 }
diff --git a/compass_app/app/lib/data/repositories/booking/booking_repository_local.dart b/compass_app/app/lib/data/repositories/booking/booking_repository_local.dart
@@ -31,7 +31,7 @@ class BookingRepositoryLocal implements BookingRepository {
     // Bookings created come without id, we need to assign one
     final bookingWithId = booking.copyWith(id: _sequentialId++);
     _bookings.add(bookingWithId);
-    return Result.ok(null);
+    return const Result.ok(null);
   }
 
   @override
@@ -92,6 +92,6 @@ class BookingRepositoryLocal implements BookingRepository {
   @override
   Future<Result<void>> delete(int id) async {
     _bookings.removeWhere((booking) => booking.id == id);
-    return Result.ok(null);
+    return const Result.ok(null);
   }
 }
diff --git a/compass_app/app/lib/data/repositories/itinerary_config/itinerary_config_repository_memory.dart b/compass_app/app/lib/data/repositories/itinerary_config/itinerary_config_repository_memory.dart
@@ -22,6 +22,6 @@ class ItineraryConfigRepositoryMemory implements ItineraryConfigRepository {
     ItineraryConfig itineraryConfig,
   ) async {
     _itineraryConfig = itineraryConfig;
-    return Result.ok(true);
+    return const Result.ok(true);
   }
 }
diff --git a/compass_app/app/lib/data/services/api/api_client.dart b/compass_app/app/lib/data/services/api/api_client.dart
@@ -53,7 +53,7 @@ class ApiClient {
         return Result.ok(
             json.map((element) => Continent.fromJson(element)).toList());
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -74,7 +74,7 @@ class ApiClient {
         return Result.ok(
             json.map((element) => Destination.fromJson(element)).toList());
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -97,7 +97,7 @@ class ApiClient {
             json.map((element) => Activity.fromJson(element)).toList();
         return Result.ok(activities);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -119,7 +119,7 @@ class ApiClient {
             json.map((element) => BookingApiModel.fromJson(element)).toList();
         return Result.ok(bookings);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -139,7 +139,7 @@ class ApiClient {
         final booking = BookingApiModel.fromJson(jsonDecode(stringData));
         return Result.ok(booking);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -160,7 +160,7 @@ class ApiClient {
         final booking = BookingApiModel.fromJson(jsonDecode(stringData));
         return Result.ok(booking);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -180,7 +180,7 @@ class ApiClient {
         final user = UserApiModel.fromJson(jsonDecode(stringData));
         return Result.ok(user);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
@@ -197,9 +197,9 @@ class ApiClient {
       final response = await request.close();
       // Response 204 "No Content", delete was successful
       if (response.statusCode == 204) {
-        return Result.ok(null);
+        return const Result.ok(null);
       } else {
-        return Result.error(const HttpException("Invalid response"));
+        return const Result.error(HttpException("Invalid response"));
       }
     } on Exception catch (error) {
       return Result.error(error);
diff --git a/compass_app/app/lib/data/services/api/auth_api_client.dart b/compass_app/app/lib/data/services/api/auth_api_client.dart
@@ -32,7 +32,7 @@ class AuthApiClient {
         final stringData = await response.transform(utf8.decoder).join();
         return Result.ok(LoginResponse.fromJson(jsonDecode(stringData)));
       } else {
-        return Result.error(const HttpException("Login error"));
+        return const Result.error(HttpException("Login error"));
       }
     } on Exception catch (error) {
       return Result.error(error);
diff --git a/compass_app/app/lib/data/services/shared_preferences_service.dart b/compass_app/app/lib/data/services/shared_preferences_service.dart
@@ -32,7 +32,7 @@ class SharedPreferencesService {
         _log.finer('Replaced token');
         await sharedPreferences.setString(_tokenKey, token);
       }
-      return Result.ok(null);
+      return const Result.ok(null);
     } on Exception catch (e) {
       _log.warning('Failed to set token', e);
       return Result.error(e);
diff --git a/compass_app/app/lib/domain/use_cases/booking/booking_create_use_case.dart b/compass_app/app/lib/domain/use_cases/booking/booking_create_use_case.dart
@@ -98,7 +98,7 @@ class BookingCreateUseCase {
       case Ok<List<Destination>>():
         final destination = result.value
             .firstWhere((destination) => destination.ref == destinationRef);
-        return Ok(destination);
+        return Result.ok(destination);
       case Error<List<Destination>>():
         return Result.error(result.error);
     }
diff --git a/compass_app/app/lib/domain/use_cases/booking/booking_share_use_case.dart b/compass_app/app/lib/domain/use_cases/booking/booking_share_use_case.dart
@@ -37,7 +37,7 @@ class BookingShareUseCase {
     try {
       await _share(text);
       _log.fine('Shared booking');
-      return Result.ok(null);
+      return const Result.ok(null);
     } on Exception catch (error) {
       _log.severe('Failed to share booking', error);
       return Result.error(error);
diff --git a/compass_app/app/lib/ui/booking/view_models/booking_viewmodel.dart b/compass_app/app/lib/ui/booking/view_models/booking_viewmodel.dart
@@ -61,7 +61,7 @@ class BookingViewModel extends ChangeNotifier {
             _log.fine('Created Booking');
             _booking = result.value;
             notifyListeners();
-            return Result.ok(null);
+            return const Result.ok(null);
           case Error<Booking>():
             _log.warning('Booking error: ${result.error}');
             notifyListeners();
diff --git a/compass_app/app/lib/utils/result.dart b/compass_app/app/lib/utils/result.dart
@@ -18,11 +18,11 @@
 sealed class Result<T> {
   const Result();
 
-  /// Creates an instance of Result containing a value
-  factory Result.ok(T value) => Ok(value);
+  /// Creates a successful [Result], completed with the specified [value].
+  const factory Result.ok(T value) = Ok._;
 
-  /// Create an instance of Result containing an error
-  factory Result.error(Exception error) => Error(error);
+  /// Creates an error [Result], completed with the specified [error].
+  const factory Result.error(Exception error) = Error._;
 
   /// Convenience method to cast to Ok
   Ok<T> get asOk => this as Ok<T>;
@@ -33,7 +33,7 @@ sealed class Result<T> {
 
 /// Subclass of Result for values
 final class Ok<T> extends Result<T> {
-  const Ok(this.value);
+  const Ok._(this.value);
 
   /// Returned value in result
   final T value;
@@ -44,7 +44,7 @@ final class Ok<T> extends Result<T> {
 
 /// Subclass of Result for errors
 final class Error<T> extends Result<T> {
-  const Error(this.error);
+  const Error._(this.error);
 
   /// Returned error in result
   final Exception error;
diff --git a/compass_app/app/testing/fakes/services/fake_auth_api_client.dart b/compass_app/app/testing/fakes/services/fake_auth_api_client.dart
@@ -11,7 +11,7 @@ class FakeAuthApiClient implements AuthApiClient {
   @override
   Future<Result<LoginResponse>> login(LoginRequest loginRequest) async {
     if (loginRequest.email == 'EMAIL' && loginRequest.password == 'PASSWORD') {
-      return Result.ok(const LoginResponse(token: 'TOKEN', userId: '123'));
+      return const Result.ok(LoginResponse(token: 'TOKEN', userId: '123'));
     }
     return Result.error(Exception('ERROR!'));
   }

Original file line number	Diff line number	Diff line change
`@@ -16,12 +16,12 @@ class AuthRepositoryDev extends AuthRepository {`
`16`	`16`	`required String email,`
`17`	`17`	`required String password,`
`18`	`18`	`}) async {`
`19`		`- return Result.ok(null);`
	`19`	`+ return const Result.ok(null);`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`/// Logout is always successful in dev scenarios`
`23`	`23`	`@override`
`24`	`24`	`Future<Result<void>> logout() async {`
`25`		`- return Result.ok(null);`
	`25`	`+ return const Result.ok(null);`
`26`	`26`	`}`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ class BookingRepositoryLocal implements BookingRepository {`
`31`	`31`	`// Bookings created come without id, we need to assign one`
`32`	`32`	`final bookingWithId = booking.copyWith(id: _sequentialId++);`
`33`	`33`	`_bookings.add(bookingWithId);`
`34`		`- return Result.ok(null);`
	`34`	`+ return const Result.ok(null);`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`@override`
`@@ -92,6 +92,6 @@ class BookingRepositoryLocal implements BookingRepository {`
`92`	`92`	`@override`
`93`	`93`	`Future<Result<void>> delete(int id) async {`
`94`	`94`	`_bookings.removeWhere((booking) => booking.id == id);`
`95`		`- return Result.ok(null);`
	`95`	`+ return const Result.ok(null);`
`96`	`96`	`}`
`97`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,6 @@ class ItineraryConfigRepositoryMemory implements ItineraryConfigRepository {`
`22`	`22`	`ItineraryConfig itineraryConfig,`
`23`	`23`	`) async {`
`24`	`24`	`_itineraryConfig = itineraryConfig;`
`25`		`- return Result.ok(true);`
	`25`	`+ return const Result.ok(true);`
`26`	`26`	`}`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ class AuthApiClient {`
`32`	`32`	`final stringData = await response.transform(utf8.decoder).join();`
`33`	`33`	`return Result.ok(LoginResponse.fromJson(jsonDecode(stringData)));`
`34`	`34`	`} else {`
`35`		`- return Result.error(const HttpException("Login error"));`
	`35`	`+ return const Result.error(HttpException("Login error"));`
`36`	`36`	`}`
`37`	`37`	`} on Exception catch (error) {`
`38`	`38`	`return Result.error(error);`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ class SharedPreferencesService {`
`32`	`32`	`_log.finer('Replaced token');`
`33`	`33`	`await sharedPreferences.setString(_tokenKey, token);`
`34`	`34`	`}`
`35`		`- return Result.ok(null);`
	`35`	`+ return const Result.ok(null);`
`36`	`36`	`} on Exception catch (e) {`
`37`	`37`	`_log.warning('Failed to set token', e);`
`38`	`38`	`return Result.error(e);`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ class BookingCreateUseCase {`
`98`	`98`	`case Ok<List<Destination>>():`
`99`	`99`	`final destination = result.value`
`100`	`100`	`.firstWhere((destination) => destination.ref == destinationRef);`
`101`		`- return Ok(destination);`
	`101`	`+ return Result.ok(destination);`
`102`	`102`	`case Error<List<Destination>>():`
`103`	`103`	`return Result.error(result.error);`
`104`	`104`	`}`