Improve "Pub Points" on pub.dev about openfoodfacts-dart HOT 25 CLOSED

I've just run pana on the latest version of the code, and got a score of 90/100. The current score on pub.dev is 70/110:

• it looks like with the recent changes we earned 20 points :)
• the probable pub.dev score for the next version is 90/110
• in pana, the 10 points we miss is because we don't support null-safety (which is only in beta). I think we should play it safe and wait until dart 2.12 is stable - then we'll be able to migrate to null-safety.
• why 100 in pana and 110 in pub.dev ? It looks like in pana they don't compute the 10-point score for the comments. So far we have dart comments only on 2.6%, and the threshold is 20%...

0/10 points: 20% or more of the public API has dartdoc comment
41 out of 1598 API elements (2.6 %) have documentation comments.
Providing good documentation for libraries, classes, functions, and other API elements improves code readability and helps developers find and use your API. Document at least 20% of the public API elements.

from openfoodfacts-dart.

I just updated the packages to the newest version #98.
If I haven't missed any, all the packages we use already support null safety. That means we are ready for the first test.🎉

from openfoodfacts-dart.

We did it, we reached the 130/130 score 🥇

Thanks @monsieurtanuki for your provided mass documentation, we reached a stunning 46.7 % coverage

from openfoodfacts-dart.

Wonderful! Well, it took about 8 PRs to go from 80 to 130 points. I guess now we have enough code stability to stay at 130.

from openfoodfacts-dart.

Currently working on the "Pass static analysis" score (for 0.3.15+2: 20/30)

from openfoodfacts-dart.

Hi @monsieurtanuki,
If I recall I had to overwrite the generated factory in order to keep the products in the Json format as well as a list of objects. The json_annotation plugin doesn't allow to use a field multiple times. There is maybe a way to do it but I didn't find it at the time.
Also I can't remember why we had to keep both formats, maybe it is not the case anymore

from openfoodfacts-dart.

That's fine. By all means remove it if everyone else agrees. It's not that I'm a fan of json_serializable I'm just pointing out a significant cost of not using it. I always felt that JSON conversion is a bit awkward in Dart as compared to languages that support reflection.

from openfoodfacts-dart.

I agree with you @MohamedFBoussaid, no rush.
As for the tests of the null-safety version, I think it will be a good opportunity for an improvement of the unit test code on a broader range (if needed). We'll see then.

from openfoodfacts-dart.

I don't know how pub.dev counts the API elements; nevertheless I've roughly counted the number of lines of each file in the Android Studio "structure" tab and found about 1700 - compared to the 1859 indicated by pub.dev.

In order to exceed the 20% threshold, I'm going to be lazy and just comment stupidly the languages (186) and the nutrients (170): we should reach around 28% of comments, and we'll get the 130/130 score. Hopefully...

from openfoodfacts-dart.

That's a great idea.

from openfoodfacts-dart.

Thanks @monsieurtanuki

from openfoodfacts-dart.

Hi @PrimaelQuemerais!
There's something fishy in your code in SearchResult.dart (compared to the generated SearchResult.g.dart)
The expected (and previous version) of method fromJson is:

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

But you changed it months ago, and now _$SearchResultFromJson(json) is never called. And its code is slightly different from the current implementation of factory SearchResult.fromJson(Map<String, dynamic> json).
In the pub.dev score, we get a 10 point penalty because _$SearchResultFromJson(json) is never used. And it's also strange that the code is different. Could you please have a look at it? If you do that I'll give you 10 points :)

from openfoodfacts-dart.

Hi @Grumpf!
In HttpHelper.dart you use List<int> fileBytes = await File.fromUri(entry.value).readAsBytes(); to get the content of a Uri. Nothing that really shocks me, except that using File means using dart.io, which prevents developer from using openfoodfacts with flutter web. And it costs us 10 points.
I'm sure there's another way to do that; what about this:

http.Response response = await http.get(entry.value);
List<int> fileBytes = response?.bodyBytes;

Could you please have a look at it? If you do that I'll give you 10 points, fair is fair :)

from openfoodfacts-dart.

Well, not everything is clear in my mind now, but we're getting closer to 110 points.

We lose 10 points because of this: "At least 20% of the public API members contain API documentation."

And there's something I don't get about http: ^1.3.0 that we're supposed to support though it's not the stable version... 10 points again...

For the record:

• pana: A library for analyzing Dart packages
• help about scoring

from openfoodfacts-dart.

Thank you @PrimaelQuemerais for your answer.
I don't know either why both formats are kept ("for historical reasons"), but anyway it takes space to keep both, given that we can compute the json string from the json structure (the latter being the more interesting version I guess).

I think the correct way out of this would be

• to consider final List<Product> products as the only interesting field (from/to json)
• to transform final List<dynamic> jsonProducts into a deprecated getter
• or to get rid of the json annotation for this class, but I'm sure @peterwvj would not agree with that ;)

from openfoodfacts-dart.

The right thing to do would be to get rid of jsonProducts. It seems wrong to have it. I noticed that it is being used in openfoodfacts.dart. I'm not sure why but it looks odd (I marked the line in the code snippet below):

  static Future<SearchResult> queryPnnsGroup(
      User user, PnnsGroupQueryConfiguration configuration,
      {QueryType queryType = QueryType.PROD}) async {
    const outputFormat = OutputFormat(format: Format.JSON);
    var queryParameters = configuration.getParametersMap();
    queryParameters[outputFormat.getName()] = outputFormat.getValue();

    var searchUri = Uri(
        scheme: URI_SCHEME,
        host: queryType == QueryType.PROD ? URI_PROD_HOST : URI_TEST_HOST,
        path: '/pnns-group-2/${configuration.group.id}/${configuration.page}',
        queryParameters: queryParameters);

    Response response = await HttpHelper()
        .doGetRequest(searchUri, user: user, queryType: queryType);
    var result = SearchResult.fromJson(json.decode(response.body));

    if (configuration.fields
            .contains(ProductField.CATEGORIES_TAGS_TRANSLATED) ||
        configuration.fields.contains(ProductField.LABELS_TAGS_TRANSLATED) ||
        configuration.fields.contains(ProductField.ALL)) {
      result.products.asMap().forEach((index, product) {
        ProductHelper.removeImages(product, configuration.language);
        ProductHelper.addTranslatedFields(product,
            result.jsonProducts.elementAt(index), configuration.language); // USED HERE !!!!!!!!!
      });
    } else {
      result.products.asMap().forEach((index, product) {
        ProductHelper.removeImages(product, configuration.language);
      });
    }

    return result;
  }

from openfoodfacts-dart.

A zoom on ProductHelper.addTranslatedFields:

  static void addTranslatedFields(Product product, Map<String, dynamic> source,
      OpenFoodFactsLanguage language) {
    product.categoriesTagsTranslated =
        source['categories_tags_${language.code}'];
    product.labelsTagsTranslated = source['labels_tags_${language.code}'];
  }

The idea seems to be: only if some flags are set (CATEGORIES_TAGS_TRANSLATED, LABELS_TAGS_TRANSLATED, ALL), there's an additional step that computes 2 translated fields (according to the language in parameter) (that we already know when we run the search).
The problem is that we keep the double of the needed space of each Product (around 10Kb) just for that...
A solution would be to store all the 'categories_tags_${language.code}' when we decode the json, in a Map. Then, we'll be able to retrieve the labels for each desired language, from that Map. That would cost less in terms of space.

from openfoodfacts-dart.

Thanks for looking into this. Yes, that would be much better than storing all the products twice.

from openfoodfacts-dart.

Of course I could easily add that Map in Product.dart, but I would do that my way, which means removing the json annotations.

@peterwvj Could you add (and populate) that Map with minor changes while keeping the json annotations? Are those annotations flexible enough?

from openfoodfacts-dart.

Feel free to remove the annotation from SearchResult, if you prefer.

from openfoodfacts-dart.

@peterwvj The action must be more specifically on Product, where the categories and their possibly translated labels are. If I only remove the field SearchResult.jsonProducts I lose the translations.

from openfoodfacts-dart.

Okay, thanks for clarifying. Product.dart is huge and without json_serializable JSON conversion for this class would require a lot of manually written code, I think. I not sure it's worthwhile to be honest.

from openfoodfacts-dart.

@peterwvj Json annotations are unfortunately:

• not flexible enough for our needs in Product
• force us to waste space at runtime (about additional 10Kb per Product) (and where I come from it's a lot)
• blur code clarity in SearchResult

We'll still enjoy json annotations for other classes, and we'll use what json annotations generated for Product in the one-shot transition.

from openfoodfacts-dart.

@monsieurtanuki a great job was done there thanks, and it is a huge improvement for the score.
And for the null safety, I have tried to make the package null safety, but it is too risky like walking on a minefield. When the time comes to move to null safety we should create a internal version that we can test first (I don't if it is possible with pub.dv to have a version with restricted access)

from openfoodfacts-dart.

Version 1.3.0 - still some work to be done for a score of 130/130:

170 out of 1859 API elements (9.1 %) have documentation comments.
Providing good documentation for libraries, classes, functions, and other API elements improves code readability and helps developers find and use your API. Document at least 20% of the public API elements.

from openfoodfacts-dart.