feat: #132 - new method OpenFoodAPIClient.addProductFields

[monsieurtanuki]

New files:

• AddableProductFields.dart: Fields of a Product that can be simply added.
• api_addProductFields_test.dart: Unit/Integration tests around OpenFoodAPIClient.addProductFields

Impacted file:

• openfoodfacts.dart: new method addProductFields

What

• new method OpenFoodAPIClient.addProductFields

Part of

• feat: Add basic details page UI smooth-app#1515

[monsieurtanuki]

@g123k ping

[g123k]

By curiosity, why don't you call Map.from(user.toData)?

[g123k]

A StringBuffer would be better, since you concatenate strings multiple times

[monsieurtanuki]

@g123k StringBuffer is never used in the whole project. Here we're talking about adding a String to a String twice, and checking the resulting value 3 times (that would mean 3 additional toString() calls with StringBuffer). In a test. That makes async calls to the back-end that take longer time than a String allocation.

[monsieurtanuki]

@g123k I had a hunch about StringBuffer not being significantly faster than String when you only concatenate two Strings and convert each time the result as a String.
I've coded a test with the 2 solutions: with String and with StringBuffer.

test code

      const String initialString = "init";
      const List<String> additionalValues = <String>['djobi', 'djoba'];
      final List<String> result = <String>[];

      late int start;
      late int end;

      result.clear();
      start = DateTime.now().microsecondsSinceEpoch;
      final StringBuffer asStringBuffer = StringBuffer(initialString);
      result.add(asStringBuffer.toString());
      for (final String additionalValue in additionalValues) {
        asStringBuffer.write(', $additionalValue');
        result.add(asStringBuffer.toString());
      }
      end = DateTime.now().microsecondsSinceEpoch;
      print('${end - start}: $result');

      result.clear();
      start = DateTime.now().microsecondsSinceEpoch;
      String asString = initialString;
      result.add(asString);
      for (final String additionalValue in additionalValues) {
        asString += ', $additionalValue';
        result.add(asString);
      }
      end = DateTime.now().microsecondsSinceEpoch;
      print('${end - start}: $result');

After several runs, I've noticed that the first block run is a bit slower than the other - regardless of StringBuffer and String. By approximately 1ms.
Can you afford that 1ms uncertainty for tests? I think so. If that's OK with you, please approve my PR. If you think there is a hidden performance potential, don't hesitate to keep this PR rejected and to run performance tests.

[monsieurtanuki]

@g123k "Requested changes" don't look like an appropriate term to me in that case: "By curiosity", "would be better". We're talking about mere comments.
At least the way I see things, "Requested changes" is a bazooka when something problematic happens in the code, like obvious use-cases not taken into account or daft copy/pastes.

[g123k]

@g123k "Requested changes" don't look like an appropriate term to me in that case: "By curiosity", "would be better". We're talking about mere comments. At least the way I see things, "Requested changes" is a bazooka when something problematic happens in the code, like obvious use-cases not taken into account or daft copy/pastes.

True, but this one requires a change

[M123-dev]

Ready to merge?

[monsieurtanuki]

I'm afraid not.