Product.attributeGroups is now a List<AttributeGroup>

[monsieurtanuki]

No description provided.

[monsieurtanuki]

@peterwvj With the minimum impact on Product.

[monsieurtanuki]

This PR would also solve this new issue detected by @M123-dev (openfoodfacts/smooth-app#55 (comment)):

E/flutter (15852): [ERROR:flutter/lib/ui/ui_dart_state.cc(177)] Unhandled Exception: FormatException: Invalid double
E/flutter (15852): null
E/flutter (15852): #0      double.parse (dart:core-patch/double_patch.dart:111:28)
E/flutter (15852): #1      AttributeGroups.fromJson (package:openfoodfacts/model/AttributeGroups.dart:67:27)
E/flutter (15852): #2      _$ProductFromJson (package:openfoodfacts/model/Product.g.dart:71:25)
E/flutter (15852): #3      new Product.fromJson (package:openfoodfacts/model/Product.dart:180:7)
E/flutter (15852): #4      _$ProductResultFromJson (package:openfoodfacts/model/ProductResult.g.dart:16:19)
E/flutter (15852): #5      new ProductResult.fromJson (package:openfoodfacts/model/ProductResult.dart:20:7)
E/flutter (15852): #6      OpenFoodAPIClient.getProduct (package:openfoodfacts/openfoodfacts.dart:143:42)

[peterwvj]

@monsieurtanuki thanks for working on this. I tested this PR with my app and it seems to work. I'm willing to accept the PR but maybe someone with more knowledge on this topic should take a look as well.

I still doubt whether making the fields in Attribute and AttributeGroup final is a good idea.

[monsieurtanuki]

@peterwvj Thank you for your positive review.

About making the fields final, I would say after 9 months of flutter that it's the best practice. flutter keeps creating objects again and again (think of the Widgets), that's in its DNA. In other programing languages the philosophy can be different.
I'm a big fan of the final keyword in general: anything that can prevent me from coding something wrong is welcome (like a value that I unexpectedly changed in the middle).
In that specific case (AttributeGroup and Attribute), I think we're rather in a read-only mode, getting data from the API. Will that always be the case? I don't know. We'll change that later if needed.

AFAIK we don't have a clear best practice cheat sheet in this project about how to handle those cases.
For instance, in Product, barcode is not final: does that make sense? Some fields are in the constructor, other not: is there a logic behind that or was it just the way things evolved after contributions of different contributors with different backgrounds?

[peterwvj]

@monsieurtanuki I agree with your points. The only advantage you get by not making the fields final is that you can instantiate objects and gradually set the fields. This can be helpful during testing (some of my app tests use the Product class this way), i.e. creating objects, performing some tests, setting some fields, running some more tests and so on.

@PrimaelQuemerais would you mind taking a look at this PR in order to make sure that you agree with the changes? I already reviewed the PR and approved the changes but it would be nice if you could take a look as well. Thanks in advance.

[PrimaelQuemerais]

I merged the PR with the final fields for now. I don't see any case were we would need to write an Attribute or AttributeGroup to the API at the moment. I just had to update some test due to changes in the field values returned by the API.
If the final fields are proven to be beneficial we will add it to the best practice documentation.

Changes are live in version 0.3.14+1 of the plugin on pub.dev.
Thanks for the PR @monsieurtanuki and for the review @peterwvj 🙂