-
-
Notifications
You must be signed in to change notification settings - Fork 287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Code base cleanup #808
Code base cleanup #808
Conversation
@@ -69,8 +71,10 @@ class _ImageUploadCardState extends State<ImageUploadCard> { | |||
User(userId: 'smoothie-app', password: 'strawberrybanana'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teolemon this is one of two hardcoded app users, this even added two test (non food) products to the DB, when I remeber right we don't even need them anymore since we do not longer need a user to add a photo, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "we do not longer need a user to add a photo" ?
- We have never formally needed a user for photos (it's attributed to openfoodfacts-contributors). I believe @stephanegigandet created a specific user to see how efficient Smoothie was at gathering photos.
- We still have the case where the user is not logged in, and where we could use that user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @M123-dev!
You can't have that many users:
const User myUser = User(userId: 'smoothie-app', password: 'strawberrybanana');
static const User SMOOTH_USER = User(
userId: 'project-smoothie',
password: 'smoothie',
comment: 'Test user for project smoothie',
);
OpenFoodAPIConfiguration.globalUser ?? myUser
OpenFoodAPIConfiguration.globalUser ?? ProductQuery.SMOOTH_USER
My suggestions:
- remove one of the default users
- make the other one private, in
ProductQuery
- add the following method in
ProductQuery
:
static User getUser() => OpenFoodAPIConfiguration.globalUser ?? _defaultUser;
- From now on use only
ProductQuery.getUser()
Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@M123-dev That's exactly what I had in mind, except for the name.
For consistency reasons with getLanguage
and getCountry
, getUser
makes more sense, doesn't it?
If you have time, please consider either
- methods named
getLanguage()
,getCountry()
andgetUser()
- or getters named
language
,country
anduser
No description provided.