openfoodfacts-dart
openfoodfacts-dart copied to clipboard
Further splitting up of openfoodfacts.dart
As said in #158 openfoodfacts.dart
has gotten really big overtime there therefore we decided (for the time beeing) to code the folksonomy engine API into a separate file.
I would suggest that we take this even further. Namely that we move the Robotoff methods to a separate file and separate class like RobotoffApiClient
or so and depending on what the opinions are here do it like most dart packages I have seen and use the main file only for exports. This would mean that we move the OpenFoodAPIClient class to its own file. This should not even break anything since the package user only imports the main file with all the exports.
@monsieurtanuki @teolemon @peterwvj what are your opinions on that?
I fully support this. Even if it requires breaking changes.
Currently openfoodfacts.dart
weighs 1092 lines of code.
Inside this file, there are 6 robotoff methods (200 lines of code). That means, still 900 lines of code in openfoodfacts.dart
if we move away robotoff.
I don't know the exact relationships between OFF, robotoff and folksonomy, and if I decided to create a separate file for folksonomy it was because it seemed to me like a satellite project with different users (maybe I'm wrong). Then that made sense to create a distinct file in the same library. A different library would have been OK too. But if for the library user we're playing - for both OFF and robotoff - with the same data and the same OFF users, I'm not sure it's a good idea to split OFF and robotoff.
I mean, regardless of technical questions on our side, what would make sense for the library user? OFF, robotoff and folksonomy in the same class/file? Or would that bring confusion to mix unrelated methods? OFF, robotoff and folksonomy in different classes/files? Or would that add complexity to library users: after all, should they care - when they code in dart - if for historical reasons the subdomain is different on the server side? That was for the library user side.
Regarding "our" technical side, I'm OK with @M123-dev's suggestions. I'm a bit reluctant with a breaking change, but that would be a good opportunity to test the feat!:
keyword and to switch to version 2.0.0
.
I would even wait a little longer with the breaking changes. That is, until we have finished most of the restructuring, which hopefully won't take that long. Then we can remove all other pending deprecated fields directly with it.
I understand your point @monsieurtanuki but somehow Robotoff is still a bit different even though it uses the same users. All the methods still have Robotoff in their name. It's probably not helpfull that robotoff is found by sdk users but classes are not designed for that anyway.
This morning I can
- create
robotoff.dart
that containsOpenFoodAPIClient
's robotoff's 6 methods under new classRobotoffAPIClient
- keep the 6 methods in
OpenFoodAPIClient
but link them toRobotoffAPIClient
and deprecate them
@M123-dev OK with that?
- No strong opinion on the matter.
- Robotoff is the ML middleware of OFF, to get and validate suggestions on a given barcode
- Folksonomy is key-value model to allow adding more data to products, beyond our traditional model, but it's a separate server from OFF (it will also be extremely useful to model things like electronic devices for Open Products Facts)
Folksonomy is key-value model to allow adding more data to products, beyond our traditional model, but it's a separate server from OFF (it will also be extremely useful to model things like electronic devices for Open Products Facts)
@teolemon Is that the same Folksonomy database for food, cosmetic and pets, and potentially electronic devices, all based on a non-checked barcode? If so it should be a different library.
From @atharv028's #723 robotoff now has a proper file. No breaking change for the moment, just deprecated methods.