openfoodfacts / openfoodfacts-dart

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

OpenFoodAPIConfiguration.globalUriProductHelper ? #813

Open mopi1402 opened 11 months ago

mopi1402 commented 11 months ago

Good evening,

I am not too convinced by the uriProductHelper that takes the default value "prod" for each API.

2 use cases:

Being able to change the URLs to hit a local server or other is great, but I would have still kept something like this:

OpenFoodAPIConfiguration.globalUriProductHelper

Then, for each API, I would have put in priority order:

monsieurtanuki commented 11 months ago

Hi @mopi1402!

I understand your use-cases.

We had to change our way of dealing with "root urls" for a new feature: being able to check on another server if the product is there ("it's a food app, but perhaps the user mistakenly scanned a product that is already in open beauty facts").

Currently, you can set a static uriProductHelper of yours at app init stage, and pass it as parameter for each method.

If I had to go in your direction, I would totally remove the default value, and force developers to always explicitly set the root url.

Actually I had in mind something different: stop playing with static fields (which are rarely acclaimed as "best practices" in OOP) and merge OpenFoodAPIConfiguration into OpenFoodAPIClient (that would not be made of static methods anymore either). Like, "I have an instance of OpenFoodAPIClient and I call methods for this instance". Or something like that. But currently I have other priorities, and it would only be a clean OOP refactoring without much added value, feature-wise. And I would like to see what we do with open beauty/pet food/product facts beforehand.

Does that make sense?

M123-dev commented 11 months ago

@monsieurtanuki from an OOP standpoint it makes total sense, but it would mean a VERY big breaking change, until then I understand the wish to not have manually pass it to every method, most sdk users won't care about which URL to be used

monsieurtanuki commented 11 months ago

@M123-dev I have no intention to switch from static to instance methods in short term. Again, that would rather make sense when (if?) we have a clear view about OBF OPF OPFF. Which is currently not the case.