tat_flutter icon indicating copy to clipboard operation
tat_flutter copied to clipboard

遷移 Course Connector 至 v3 與重構 Course Connector 實作細節

Open ntut-xuan opened this issue 1 year ago • 1 comments

Description

Warning:這個 PR 是重構相關 PR, diff 過大非常抱歉,僅需確認功能性正常。

在這個 PR 上,我遷移了 CourseExtraJson 與其相關無用的 Json 架構,使 Course 與 Syllabus 來涵蓋大部分的無用 Json,並且遷移了 Course Connector 至 v3

Implementation

  • [x] Apply Course Object.
  • [x] Migrate course connector to dart v3.

Testing Instructions

  • [x] Test course table related function work properly.

ntut-xuan avatar Feb 08 '24 17:02 ntut-xuan

@ntut-xuan For this comment: https://github.com/NEO-TAT/tat_flutter/pull/221#discussion_r1484952980

Based on our discussions, it seems that we need not only using stricter conversion for both num and List<String> type of the Course model, but also late-avoided constant class syntax, there might be a much feasible solution. let's discuss in the following steps.

A better approach for replacing static-member class JsonInit

In json_serializable, there's actually an advance tool called JsonConverter described in here. The JsonConverter has the following prototype:

abstract class JsonConverter<T, S> {
  const JsonConverter();

  T fromJson(S json);
  S toJson(T object);
}

which allows as to implement our custom json conversion rules and use it as an annotation to specific field of the model class. Here the type S is what we expect the original type from json raw data, while the type T is what we going to get after conversion.

Hence, I'd suggest using this approach for better coding structure and readability. Let's look each of the real case in the following sections.

List<String> conversion

It seems that the API response data will give us something like this foo \nbar \n and you expect to get these two strings separately then put them into a List<String> so that you tried to use regexp to remove all new-line char \n then trim() them.

So that means we can implement a custom converter like this:

class _SplitByNewLineConverter implements JsonConverter<List<String>, String?> {
  const _SplitByNewLineConverter();

  @override
  List<String> fromJson(String? json) {
    if (json == null) {
      return const [];
    }

    return json.trim().split(r'\n').map((element) => element.trim()).toList();
  }

  @override
  String? toJson(List<String> object) => object.join('\n');
}

In this implementation, we can put our special check and separation logics into fromJson (since they are not too big) directly.

Then, we add this annotation to our field which requires this conversion:

...
  @_SplitByNewLineConverter()
  final List<String> teachers;

  @_SplitByNewLineConverter()
  final List<String> classNames;

  @_SplitByNewLineConverter()
  final List<String> classrooms;
...

I use String? as the source json data type because the raw data is definitely a string data at the first time from the response, additionally, use nullable type to state that this field may probably not given.

Using a nullable type in a model can clearly show that this thing is "not exist" instead of "not assigned". So for example, if there's a field called foo in the model but the api response doesn't contains this field, which means there's no "foo": "..." in the raw response json, foo should be null instead of "" (empty string) because if the api response contains "foo": "", it will conflicts with the previous case. (This is why we use null here!)

So if we use this annotation, the generated .g.dart file will looks like this:

      teachers: const _SplitByNewLineConverter().fromJson(json['teachers'] as String?),
      classNames: const _SplitByNewLineConverter().fromJson(json['classNames'] as String?),
      classrooms: const _SplitByNewLineConverter().fromJson(json['classrooms'] as String?),

which calls our custom converter function to process the data.

int & double conversion

For converting these two types here, I saw you implemented the following logics:

  static int intInit(String value) {
    return value.replaceAll(RegExp(r"\s"), "").isNotEmpty ? int.parse(value) : 0;
  }

  static double doubleInit(String value) {
    return value.replaceAll(RegExp(r"\s"), "").isNotEmpty ? double.parse(value) : 0.00;
  }

However, there might be another more straightforward approach.

  • Use nullable type. Like I've mentioned above, set a variable's value to the default value of its type may not seems to be a recommended approach, since this would possibly cause data misunderstandings when the value is really equal to the default value of the type.

    final double foo = getFooDataFrom(response);
    print(foo); // 0.0
    

    Is it easy to distinguish whether foo is given from server or not ?

    So I'd suggest changing their type to this:

      final int? id;
      final int? periodCount;
      final double? stage;
      final double? credit;
    
  • Use num.tryParse(...) instead If we aware that there's something not a number appears in the raw json data, we can call this method to help us done this. Here's the prototype:

      // dart 2.19
      static num? tryParse(String input) {
        String source = input.trim();
        return int.tryParse(source) ?? double.tryParse(source);
      }
    

    This method will return the actual number according to the raw json data, or a null value if there's anything cause conversion failed. So that means it helps us to check "whether there's only lots if \s in the raw data".

So based on above concepts, we can create another JsonConverter like this:

class _ExcludeSpacesInNumberConverter implements JsonConverter<num?, String?> {
  const _ExcludeSpacesInNumberConverter();

  @override
  num? fromJson(String? json) {
    if (json == null) {
      return null;
    }

    return num.tryParse(json);
  }

  @override
  String? toJson(num? object) => object?.toString();
}

However, we have both int and double case, and the json_serializable will use the custom converter only when the result type is definitely matching the field type that we declared in the model class. This means we can not use num? as our return type, since we declare our fields either int? or double?. So for clearing specify a num? should be a int? or double? we can make _ExcludeSpacesInNumberConverter become an abstract class,

abstract class _ExcludeSpacesInNumberConverter<T extends num> implements JsonConverter<T?, String?> {
  const _ExcludeSpacesInNumberConverter();

  @override
  T? fromJson(String? json) {
    if (json == null) {
      return null;
    }

    return num.tryParse(json) as T?;
  }

  @override
  String? toJson(T? object) => object?.toString();
}

then inherit it by those two classes:

class _ExcludeSpacesInIntegerConverter extends _ExcludeSpacesInNumberConverter<int> {
  const _ExcludeSpacesInIntegerConverter();

  @override
  int? fromJson(String? json) => super.fromJson(json)?.toInt();
}

class _ExcludeSpacesInDoubleConverter extends _ExcludeSpacesInNumberConverter<double> {
  const _ExcludeSpacesInDoubleConverter();

  @override
  double? fromJson(String? json) => super.fromJson(json)?.toDouble();
}

and finally, we can add annotation to our field:

  @_ExcludeSpacesInIntegerConverter()
  final int? id;

  @_ExcludeSpacesInIntegerConverter()
  final int? periodCount;

  @_ExcludeSpacesInDoubleConverter()
  final double? stage;

  @_ExcludeSpacesInDoubleConverter()
  final double? credit;

And the generated .g.dart file will looks like this:

      id: const _ExcludeSpacesInIntegerConverter().fromJson(json['id'] as String?),
      stage: const _ExcludeSpacesInDoubleConverter().fromJson(json['stage'] as String?),
      credit: const _ExcludeSpacesInDoubleConverter().fromJson(json['credit'] as String?),
      periodCount: const _ExcludeSpacesInIntegerConverter().fromJson(json['periodCount'] as String?),

That's it, I hope these concepts and ideas helps us improve the project :)


P.S. the whole code of `Course`
@JsonSerializable()
class Course {
  final String name;
  final String? category;
  final String? applyStatus;
  final String? language;
  final String? syllabusLink;
  final String? note;

  @_ExcludeSpacesInIntegerConverter()
  final int? id;

  @_ExcludeSpacesInIntegerConverter()
  final int? periodCount;

  @_ExcludeSpacesInDoubleConverter()
  final double? stage;

  @_ExcludeSpacesInDoubleConverter()
  final double? credit;

  @_SplitByNewLineConverter()
  final List<String> teachers;

  @_SplitByNewLineConverter()
  final List<String> classNames;

  @_SplitByNewLineConverter()
  final List<String> classrooms;

  final List<CoursePeriod> coursePeriods;

  const Course({
    required this.name,
    this.id,
    this.stage,
    this.credit,
    this.periodCount,
    this.category,
    this.applyStatus,
    this.language,
    this.syllabusLink,
    this.note,
    List<String>? teachers,
    List<String>? classNames,
    List<String>? classrooms,
    List<CoursePeriod>? coursePeriods,
  })  : teachers = teachers ?? const [],
        classNames = classNames ?? const [],
        classrooms = classrooms ?? const [],
        coursePeriods = coursePeriods ?? const [];

  factory Course.fromJson(Map<String, dynamic> json) => _$CourseFromJson(json);

  Map<String, dynamic> toJson() => _$CourseToJson(this);
}

class _SplitByNewLineConverter implements JsonConverter<List<String>, String?> {
  const _SplitByNewLineConverter();

  @override
  List<String> fromJson(String? json) {
    if (json == null) {
      return const [];
    }

    return json.trim().split(r'\n').map((element) => element.trim()).toList();
  }

  @override
  String? toJson(List<String> object) => object.join('\n');
}

abstract class _ExcludeSpacesInNumberConverter<T extends num> implements JsonConverter<T?, String?> {
  const _ExcludeSpacesInNumberConverter();

  @override
  T? fromJson(String? json) {
    if (json == null) {
      return null;
    }

    return num.tryParse(json) as T?;
  }

  @override
  String? toJson(T? object) => object?.toString();
}

class _ExcludeSpacesInIntegerConverter extends _ExcludeSpacesInNumberConverter<int> {
  const _ExcludeSpacesInIntegerConverter();

  @override
  int? fromJson(String? json) => super.fromJson(json)?.toInt();
}

class _ExcludeSpacesInDoubleConverter extends _ExcludeSpacesInNumberConverter<double> {
  const _ExcludeSpacesInDoubleConverter();

  @override
  double? fromJson(String? json) => super.fromJson(json)?.toDouble();
}

Xanonymous-GitHub avatar Feb 13 '24 05:02 Xanonymous-GitHub