openfoodfacts / openfoodfacts-dart

Open Food Facts API Wrapper
https://pub.dev/packages/openfoodfacts
Apache License 2.0
162 stars 66 forks source link

switch/enum/map refactoring #183

Closed monsieurtanuki closed 3 years ago

monsieurtanuki commented 3 years ago

In the project we have many enums, where each enum item is linked to a String tag. Currently the "enum value to String" conversion is typically coded with a switch statement. The code would be more compact and easier to read and maintain if we used static const Map<MyEnum, String> instead.

For instance:

  static const Map<TagType, String> _TAGS = {
    TagType.STATES: 'states',
    TagType.LABELS: 'labels',
    TagType.CATEGORIES: 'categories',
    TagType.COUNTRIES: 'countries',
    TagType.INGREDIENTS: 'ingredients',
    TagType.TRACES: 'traces',
    TagType.ADDITIVES: 'additives',
    TagType.ALLERGENS: 'allergens',
    TagType.LANGUAGES: 'languages',
    TagType.EMB_CODES: 'emb_codes',
  };
  String get key => _TAGS[this] ?? '';

instead of

  String get key {
    switch (this) {
      case TagType.STATES:
        return 'states';
      case TagType.LABELS:
        return 'labels';
      case TagType.CATEGORIES:
        return 'categories';
      case TagType.COUNTRIES:
        return 'countries';
      case TagType.INGREDIENTS:
        return 'ingredients';
      case TagType.TRACES:
        return 'traces';
      case TagType.ADDITIVES:
        return 'additives';
      case TagType.ALLERGENS:
        return 'allergens';
      case TagType.LANGUAGES:
        return 'languages';
      case TagType.EMB_CODES:
        return 'emb_codes';
      default:
        return '';
    }
  }

Am I missing something important, or should I go on with this refactoring? (about 13 classes impacted, should take 1 hour)

MohamedFBoussaid commented 3 years ago

Thanks @monsieurtanuki this is much cleaner. I guess it is safe to go this way, I am not seeing any reason to block the refactoring.

monsieurtanuki commented 3 years ago

Done by #184.