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.

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

Creating an English driver #2

Merged
merged 3 commits into from
Dec 6, 2015
Merged

Creating an English driver #2

merged 3 commits into from
Dec 6, 2015

Conversation

janhartigan
Copy link
Contributor

I'm actually a big fan of the way you've set this up, with a few minor exceptions. You set it up to be decently extensible and the architecture gave me some freedom to set up English in such a way that is optimal for its various quirks. Nice job. I'm guessing the biggest reason this is a fairly unused library is because it is made for Polish and Romanian. Since I need an English driver for this library, I made one for you.

Note that there is also a commit that modifies the transformer factory interface a bit to match how you were calling it in this test. Once I got a bit further into developing the English driver, I realized that the Amount instance actually holds a lot of information about how to do the transformation. I suspect that we could get away with simply passing a Number to the currency transformer and use the Currency and SubunitFormat integer from when the transformer is constructed. Seems a bit odd to have a distinct class called Amount, which is sort of just another word for "number".

I tried to follow your general style as closely as possible. I made a fairly extensive test suite that tests all currencies for English. Please let me know if you have any questions or comments.

@kwn
Copy link
Owner

kwn commented Dec 4, 2015

@janhartigan
First of all - a huge thank you for your support. I'll have a look at your code this weekend. This library is open for interfaces modifications, since work is still in progress. Until I tag it as a stable even big changes are welcome.

Just to give you a quick overview, I've decided to create this component, because the only library which does the same thing was created 10 years ago and looks a bit doggy.

Problems I had in here, were to create a representation of fractal part of money amount. I'm still wondering how I should build value objects.

If you want to use this component in your project I can create a tag with a stable release (0.0.1), to make your composer not complaining about stability of dependencies. Just let me know.

Thanks.

@janhartigan
Copy link
Contributor Author

@kwn cool, sounds good. I'll set up another issue to talk about my ideas for what I think is the optimal interface for using this package. It's barely different from what you have so far, but I feel it warrants a separate issue. Once we have that ironed out, we can maybe tweak the readme a bit to make it easier to figure out how to use it.

Again...huge respect for taking the time to make this library.

@janhartigan janhartigan mentioned this pull request Dec 4, 2015
@kwn
Copy link
Owner

kwn commented Dec 6, 2015

Great PR 👍 Good point with createCurrencyTransformer() in TransformerFactory interface. Totally agree about Amount class, which holds information about transformation. Merging now...

kwn added a commit that referenced this pull request Dec 6, 2015
Creating an English driver
@kwn kwn merged commit 751f157 into kwn:master Dec 6, 2015
@janhartigan janhartigan deleted the english branch December 7, 2015 18:02
kwn pushed a commit that referenced this pull request Jan 22, 2018
ppalashturov pushed a commit to ppalashturov/number-to-words that referenced this pull request Feb 21, 2023
ppalashturov pushed a commit to ppalashturov/number-to-words that referenced this pull request Mar 3, 2023
ppalashturov pushed a commit to ppalashturov/number-to-words that referenced this pull request Mar 3, 2023
ppalashturov pushed a commit to ppalashturov/number-to-words that referenced this pull request Mar 3, 2023
amici pushed a commit to amici/number-to-words that referenced this pull request Jul 23, 2023
kwn pushed a commit that referenced this pull request Sep 16, 2023
* Serbian language added
- with tests

* Serbian language corrected
- added separators, fixed exponents
- improved tests

Added Serbian language currency setup and test
- fix the gender for the unit cents
- removed the not needed class

* Added the currencies to README.md

* Serbian language - removed not needed inflector

* Serbian language, additional cleanup

* Serbian language, cleanup #2

* Fix the bad locale 2-letter code

---------

Co-authored-by: Andrej Mićić <amicic@someemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants