-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
feat: 773 - new method "getProductNameBrandQuantity" #844
Conversation
Impacted files: * `api_get_product_test.dart`: tested "abbreviated product name" fields * `product.dart`: added method `getProductNameBrandQuantity`; added "abbreviated product name" fields * `product.g.dart`: generated * `product_fields.dart`: added "abbreviated product name" fields
/// Returns the best version of a product name. | ||
/// | ||
/// cf. openfoodfacts-server/lib/ProductOpener/Products.pm | ||
String getBestProductName(final OpenFoodFactsLanguage language) { |
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.
I would prefer to return a null
value if there is no match; otherwise you force checking an empty string
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #844 +/- ##
==========================================
- Coverage 75.80% 75.64% -0.17%
==========================================
Files 224 224
Lines 7903 7941 +38
==========================================
+ Hits 5991 6007 +16
- Misses 1912 1934 +22 ☔ View full report in Codecov by Sentry. |
Closing as stale. |
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.
First of all sorry for my very irregular contributions, bout to write my finals in April and Mai, guess especially this PR somehow got lost in the notifications.
Added a small comment, otherwise good. Especially the new bestName method could be great for outside package users
if ((tmp = productNameInLanguages?[language])?.isNotEmpty == true) { | ||
return tmp!; | ||
} | ||
if ((tmp = productName)?.isNotEmpty == true) { | ||
return tmp!; | ||
} | ||
if ((tmp = genericNameInLanguages?[language])?.isNotEmpty == true) { | ||
return tmp!; | ||
} | ||
if ((tmp = genericName)?.isNotEmpty == true) { | ||
return tmp!; | ||
} | ||
if ((tmp = abbreviatedNameInLanguages?[language])?.isNotEmpty == true) { | ||
return tmp!; | ||
} | ||
if ((tmp = abbreviatedName)?.isNotEmpty == true) { | ||
return tmp!; | ||
} |
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.
Can't we do a
tmp ??= x;
tmp ??= y;
return tmp;
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 Looks tempting but we check "not null and not empty", not just "not null", therefore your suggestion would potentially return empty strings.
Thank you @M123-dev for your review, and good luck for your finals! |
What
getProductNameBrandQuantity
.Fixes bug(s)
Impacted files
api_get_product_test.dart
: tested "abbreviated product name" fieldsproduct.dart
: added methodgetProductNameBrandQuantity
; added "abbreviated product name" fieldsproduct.g.dart
: generatedproduct_fields.dart
: added "abbreviated product name" fields