JordyHers-org / Times-up-flutter

πŸ‘¨β€πŸ‘©β€πŸ‘§ 🚸 Parental Control App- For Android 🌟This Application use Native plugins to get local data such as Location and AppData to send it to the database. These information are saved in cache. In order to monitor the time spend on screen parent can then send messages and warn kids when it’s time to go to bed or do their homework. πŸ“±πŸŒŸπŸŒŸ
Apache License 2.0
119 stars 46 forks source link

chore(deps): bump firebase_storage from 11.2.4 to 11.6.0 #245

Open dependabot[bot] opened 9 months ago

dependabot[bot] commented 9 months ago

This serves as a simple explanation of what should be done versus what we should avoid doing to follow best practices. To also follow the humble object approach.

DO βœ…:

Domain Layer

class UserModel {
  final String id;
  final int age;
  final String name;

  factory UserModel.fromJson(final Map<String, Object?> json) =>
      UserModelFromJson(json);

  factory UserModel.toJson(final Map<String, Object?> json) =>
      _$UserModelFromJson(json);
  UserModel(this.id, this.age, this.name);
}

Data Layer

class APIPath {
  APIPath();
  static String company(final String uid, final String companyId) =>
      'users/$uid/company/$companyId';
  static String user(final String uid, final String companyId) => 'users/$uid';
}
abstract class ApiInterface {
  UserModel? get currentUser;
  Future<void> setUser(final UserModel model);
  Future<void> updateUser(final UserModel model);
  Future<void> deleteUser(final UserModel model);
  Stream<List<UserModel>> userStream();
}

Description:

Here the repository level implementation does not know anything about the domain layer. We have single line baring as little logic as possible. Each of the implementation is a single line of code. It remains testable, readable and scalable, making the code less bug prone.

class ApiImpl implements ApiInterface {
  factory ApiImpl({required final String uid}) {
    return _singleton ??= ApiImpl._internal(uid);
  }

  ApiImpl._internal(this.uid);
  static ApiImpl? _singleton;

  final _service = locator<AuthService>();

  @override
  UserModel? get currentUser => _user;

  @override
  Future<void> setUser(final UserModel model) => _service.setData(
        path: APIPath.user(uid, model.id),
        data: model.toJson(),
      );

  @override
  Future<void> updateUser(final UserModel model) => _service.updateData(
        path: APIPath.user(uid, model.id),
        data: model.toJson(),
      );

  @override
  Future<void> deleteUser(final UserModel model) async {
    await _service.deleteData(
      path: APIPath.user(uid, model.id),
      image: model.image,
    );
  }

  @override
  Stream<UserModel> userStream({required final String userId}) =>
      _service.documentStream(
        path: APIPath.user(uid, userId),
        builder: (final data, final documentId) => UserModel.fromJson(data),
      );

  @override
  Stream<List<UserModel>> userStream() => _service.collectionStream(
        path: APIPath.user(uid),
        builder: UserModel.fromJson,
      );
}

Service:

This file should not bear specific functions but rather. The idea of having generic method is that we do not have to test these but rather only test the implementation level.If there are break in data or issue, this layer will be out of cause.

typedef QueryBuilder<T> = T Function(Map<String, dynamic> data);

class ApiService {
  ApiService._();

  static final instance = ApiService._();

  Future<T> setData({
    required final String path,
    required final Map<String, dynamic> data,
  }) async {
    final reference = Http.instance.doc(path);
    await reference.set(data);
  }

  Future<T> updateData({
    required final String path,
    required final Map<String, dynamic> data,
  }) async {
    final reference = Http.instance.doc(path);
    await reference.update(data);
  }

  Future<T> deleteData(
      {required final String path, final String? image}) async {
    final reference = ApiService.instance.doc(path);
    if (image != null) {
      final storageReference = Http.instance.refFromURL(image);
      await storageReference.delete();
      await reference.delete();
    }
    await reference.delete();
  }

  Stream<List<T>> collectionStream<T>({
    required final String path,
    required final QueryBuilder<T> builder,
    final Function(Query query)? queryBuilder,
    final int Function(T lhs, T rhs)? sort,
  }) {
    var query = ApiService.instance.collection(path);

    if (queryBuilder != null) {
      query = queryBuilder(query) as CollectionReference<Map<String, dynamic>>;
    }
    final snapshots = query.snapshots();
    return snapshots.map((final snapshot) {
      final result = snapshot.docs
          .map((final snapshot) => builder(snapshot.data()))
          .where((final value) => value != null)
          .toList();
      if (sort != null) {
        result.sort(sort);
      }

      return result;
    });
  }

  Stream<T> documentStream<T>({
    required final String path,
    required final T Function(Map<String, dynamic> data, String documentID)
        builder,
  }) {
    final reference = Http.instance.doc(path);
    final snapshots = reference.snapshots();
    return snapshots
        .map((final snapshot) => builder(snapshot.data()!, snapshot.id));
  }
}

DO NOT DO: ❌

Domain layer

class UserModel {
  final String id;
  final int age;
  final String name;

  //❌ 1- Domain logic mixed with data parsing logic
  factory UserModel.fromJson(final Map<String, Object?> json) {
    // Domain logic (business rules)
    if (json['age'] == null || json['name'] == null) {
      throw Exception("Invalid user data");
    }

    // ❌2- Data parsing logic mixed with domain logic
    return UserModel(
      json['id'] as String,
      json['age'] as int,
      json['name'] as String,
    );
  }

  UserModel(this.id, this.age, this.name);
}

Data Layer

In the following case a common mistake is to duplicate code, mix layers and have unhandled edge case.

class ApiService {
  ApiService._();

  static final instance = ApiService._();

// ❌ - 1 Seems great but we confuse layers and throw exception
  Future<UserModel> getUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); // <----Dependency on UserModel domain class
    } else {
      throw Exception('Failed to load user');
    }
  }

  // ❌ - 2 We mix layers and repeat the same code with also handling logic.  
  Future<UserModel?> deleteUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); // <----Dependency on UserModel domain class
    } else {
      throw Exception('Failed to load user');
    }
  }
}

  // ❌ - 2 Using notify listener in wrong layer and method holds to much business logic. In 
  //addition the value returned could be null, this will cause us to cover this edge case. By 
  //trying to cover it we might cause more issues and testing that will be a nightmanre.
  Future<UserModel?> updateUser({required final String userId}) async {
    final response = await http.get('https://api.example.com/user/$userId');

    if (response.statusCode == 200) {
      // Data parsing logic mixed with domain logic
      final userData = json.decode(response.body);
      return UserModel.fromJson(userData); //
      notifyListeners(); <----Dependency on UserModel domain class
    } else if(await authService.token != null) { //<- referencing a value of different layer
         //... do some garbage here.
      throw Exception('Failed to load user');

    }
  }