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

Rewriting core logic to be (hopefully) cleaner, and address some outstanding issues #51

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

frootbirb
Copy link

Confirmed that all the unit tests succeed as before, added some new ones to account for some raised issues

@frootbirb
Copy link
Author

fixes #21
fixes #22
fixes #24
fixes #27
fixes #28
fixes #38
fixes #41
fixes #42
fixes #44
fixes #47

Unsure if this addresses #3, which is semi-duped by #22 - this does NOT split joined alpha-numeric strings like "22million", but it does handle strings like "22 million"

…shing, kids.

Also added tests to address the specific errors people listed in their issues
@frootbirb
Copy link
Author

After new updates:
fixes #30
fixes #45
fixes #48

@frootbirb frootbirb changed the title Rewriting core logic to be (hopefully) cleaner Rewriting core logic to be (hopefully) cleaner, and address some outstanding issues Nov 10, 2020
Reformatted the test cases to be more clearly divided into use cases
Added new test cases for some edge cases I forgot about
tweaked error handling to account for inconsistent behavior around decimal-only numbers and punctuation
Added support to ignore $ and made it easier to ignore other currency symbols in the future
Added support for suffixes (e.g. "$150k" -> 150000)
Added "nil" as a word option for zero
Fixed a bug where a number word followed by a period would be treated as a different number word (allowing things like "thousand thousand.")
Added support for indexing to find actual numbers as well as number words
Added new tests for new functionality
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.

1 participant