openfoodfacts / openfoodfacts-dart

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

Code generation #567

Open M123-dev opened 1 year ago

M123-dev commented 1 year ago

Description

A few days ago, I had a good idea about how we could simplify the way we have the project set up right now, mainly the problem with the Product class. We don't know what we can write back & we have to add every field not only there but also in some enums.

My solution would be code generation, in my head I thought of a big JSON file representing every field we could have, with some additional data like if it's writable and a short description so that we maintain a high documentation rate for the pub.dev score. This would even allow to create copyWith or similar function without having to write dozens of new lines of code.

What do you thing of this idea @monsieurtanuki

Tracked in

338

teolemon commented 1 year ago

We have just introduced the beginning of an OpenAPI doc in openfoodfacts-server, which should in theory allow auto-generation of SDKs

teolemon commented 1 year ago

cc @alexgarel

monsieurtanuki commented 1 year ago

@M123-dev Following our discussion on Saturday, I don't know how smart a JSON can be to describe the whole database.

If I focus on a subject that is currently quite interesting to me (Nutriments) because of pending dev (#565 #566), this is what I can say:

In addition to that, I don't believe in generated code in general, I prefer elegant code that anyone can read.

In the particular case of Nutriments, I believe we can code something very elegant (and not a class with 500+ fields). That's what I plan to do in #566.

Which does not solve the specific case of Product that triggered the OP, I'll give you that. But:

M123-dev commented 1 year ago

I totally understand your concerns about that @monsieurtanuki all those points.

But for me it's also trying out something new, regardless of if it's worth at the end or not. If I have some free time I'm going to test around with it. At first for a copyWith so that we don't have a problem with depreciation. And if it turns out to be not good we can just scrape it.

monsieurtanuki commented 1 year ago

Ok then!

stephanegigandet commented 1 year ago
  • we're a bit stuck with a contradiction - we use a JSON / Map<String, double> as input, but using a "normal class" we cannot use whatever code simplification that would bring a Map (e.g. looping on all tags)

Why don't we just do that in fact? Keep a map with string identifiers for nutrients, instead of creating specific fields for each nutrient... We can just specify a list of string constants for those identifiers so that we avoid typos. and instead of having separate fields like saturatedFat etc. we can just have getter/setter function like getNutrientValue(SATURATED_FAT, PREPARED, PER_100G).

It makes a lot of things much easier. For instance on the website, we display different nutrients in differing orders in different countries, because the default US nutrition facts table is different than the India nutrition facts table and from the EU nutrition facts table. So we can just have a list of nutrients to display for the different countries. If we don't have a getNutrientValue function and instead need to directly use fields like saturatedFat, it makes things more complex.

monsieurtanuki commented 1 year ago

@stephanegigandet Actually I'm currently implementing that for #566: