Update/NumberToWords #29
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #29 +/- ##
==========================================
- Coverage 96.02% 95.92% -0.11%
==========================================
Files 42 42
Lines 2694 2649 -45
==========================================
- Hits 2587 2541 -46
- Misses 107 108 +1
Continue to review full report at Codecov.
|
payam-zahedi
requested review from
EhsanAramide and
payam-zahedi
and removed request for
EhsanAramide
|
@EhsanAramide Why we have broken codecov task here? |
There was a problem hiding this comment.
Well done @m-khooryani, I enjoyed spending time reviewing this clean code.
Check comments and apply your changes.
| if (number <= 9) { | ||
| return _getWord(number); | ||
| } else if (number >= 11 && number <= 19) return _getWord(number); |
There was a problem hiding this comment.
I think we can make these three lines more efficient and readable. Something like:
if ( number <=19 && number!=10 ) return _getWord(number);There was a problem hiding this comment.
Also, you can write comment to make it more understandable.
| /// Gets an [int] with 3 or less digits as input and converts it to persian | ||
| String? _transformToWord(int number) { | ||
| if (number == 0) return ''; |
There was a problem hiding this comment.
I like the way this algorithm works.
| } | ||
|
|
||
| /// Returns Persian word of the given number in int You can determine | ||
| /// returned string has ordinal suffix or not by `ordinal` flag. | ||
| String numberToWords(int number, {bool ordinal = false}) { | ||
| String? numberToWords(int number, {bool ordinal = false}) { | ||
| if (number == 0) return zeroFa; |
There was a problem hiding this comment.
You have checked this condition in multiple functions. I think you can only check it in _perform function.
| } | ||
|
|
||
| /// Returns Persian word of the given number in int You can determine | ||
| /// returned string has ordinal suffix or not by `ordinal` flag. | ||
| String numberToWords(int number, {bool ordinal = false}) { | ||
| String? numberToWords(int number, {bool ordinal = false}) { |
There was a problem hiding this comment.
Is it possible for this method to return a null value? If not, we can make this null safe, and it would be a great improvement.
do you think does it have any relation with a deletion on |
I have no idea! The internal_method.dart file doesn't change in this PR. |
… removed input string extension and method, made the methods null safe
There was a problem hiding this comment.
Great work @makhosravi, You did everything good. We will try to fix the issue that we have in our CI/CD workflow. After that, we will merge this PR.
makhosravi
force-pushed
the
update/number-to-words
branch
from
1aece25 to
a1a0e9c
Compare
ali-master
added
enhancement
|
We decided to close the pull request for some errors which occur in calculating delta test coverage, because of conflicts in compared heads' SHA. |
.
Stringtype numbers to focus the feature on safeintegervalues, removed related examples and test cases