Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Swedish support for number to word conversion #139

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Add Swedish support for number to word conversion #139

merged 4 commits into from
Oct 10, 2023

Conversation

R055A
Copy link
Contributor

@R055A R055A commented Oct 1, 2023 •
edited
Loading

Fixes #120

@codecov-commenter
Copy link

Codecov Report

Merging #139 (3380c1c) into master (e9a94fd) will decrease coverage by 0.02%.
The diff coverage is 99.03%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff              @@
##             master     #139      +/-   ##
============================================
- Coverage     99.19%   99.18%   -0.02%     
- Complexity      425      447      +22     
============================================
  Files            67       72       +5     
  Lines          1736     1837     +101     
  Branches         50       56       +6     
============================================
+ Hits           1722     1822     +100     
  Misses            7        7              
- Partials          7        8       +1
Files Coverage Δ
...llegro/finance/tradukisto/LongValueConverters.java 100.00% <100.00%> (ø)
...pl/allegro/finance/tradukisto/MoneyConverters.java 100.00% <100.00%> (ø)
...pl/allegro/finance/tradukisto/ValueConverters.java 100.00% <100.00%> (ø)
...allegro/finance/tradukisto/internal/Container.java 100.00% <100.00%> (ø)
...guages/swedish/SwedishIntegerToWordsConverter.java 100.00% <100.00%> (ø)
...uages/swedish/SwedishThousandToWordsConverter.java 100.00% <100.00%> (ø)
...isto/internal/languages/swedish/SwedishValues.java 100.00% <100.00%> (ø)
...ukisto/internal/support/SwedishNumberChunking.java 100.00% <100.00%> (ø)
...languages/swedish/SwedishLongToWordsConverter.java 85.71% <85.71%> (ø)

@R055A
Copy link
Contributor Author

R055A commented Oct 2, 2023

Codecov Report

Merging #139 (3380c1c) into master (e9a94fd) will decrease coverage by 0.02%.
The diff coverage is 99.03%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff              @@
##             master     #139      +/-   ##
============================================
- Coverage     99.19%   99.18%   -0.02%     
- Complexity      425      447      +22     
============================================
  Files            67       72       +5     
  Lines          1736     1837     +101     
  Branches         50       56       +6     
============================================
+ Hits           1722     1822     +100     
  Misses            7        7              
- Partials          7        8       +1

Files Coverage Δ
...llegro/finance/tradukisto/LongValueConverters.java 100.00% <100.00%> (ø)
...pl/allegro/finance/tradukisto/MoneyConverters.java 100.00% <100.00%> (ø)
...pl/allegro/finance/tradukisto/ValueConverters.java 100.00% <100.00%> (ø)
...allegro/finance/tradukisto/internal/Container.java 100.00% <100.00%> (ø)
...guages/swedish/SwedishIntegerToWordsConverter.java 100.00% <100.00%> (ø)
...uages/swedish/SwedishThousandToWordsConverter.java 100.00% <100.00%> (ø)
...isto/internal/languages/swedish/SwedishValues.java 100.00% <100.00%> (ø)
...ukisto/internal/support/SwedishNumberChunking.java 100.00% <100.00%> (ø)
...languages/swedish/SwedishLongToWordsConverter.java 85.71% <85.71%> (ø)

Hi @jglaszka I have fixed the long value conversion to Swedish words and added more testing for this

@jglaszka jglaszka self-assigned this Oct 10, 2023
jglaszka
jglaszka approved these changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@jglaszka jglaszka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect for me, code is okay, tests covers corner cases.

src/main/java/pl/allegro/finance/tradukisto/internal/languages/swedish/SwedishValues.java

// Note: Swedish uses 'common' and 'neuter' gender types, but these seem to transfer nicely for numbers to existing
// masculine and feminine gender types for common, and neuter and non-applicable gender types for neuter.
// Regarding base numbers, only 1 has multiple gender types: 1 and 100 being neuter ('ett'), otherwise common ('en')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaination, it is really helpful

@jglaszka jglaszka merged commit 8e022d0 into allegro:master Oct 10, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jglaszka jglaszka jglaszka approved these changes

Assignees

@jglaszka jglaszka

Labels
hacktoberfest in review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Swedish
3 participants
@R055A @codecov-commenter @jglaszka