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

Simplified ConvertToArabic algorithm #733

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Simplified ConvertToArabic algorithm #733

merged 2 commits into from
Feb 2, 2024

Conversation

drademann
Copy link
Contributor

I think that your version of the conversion of roman numbers back to arabic numbers is too complicated, and distracts a bit from the main focus, the following property testing topic.

I've taken the ConvertToRoman(int) algorithm using it backwards, basically. I've updated the markdown accordingly and let my wife audit the text as her english is better than mine. But I deleted a lot of your previous tests and text of the complicated algorithm you implemented. I hope you don't mind. There might now be a gap between the test for ConvertToArabic("IV") and the final version, but I don't know how to fill it. Maybe it's not even a problem, cause there were many test steps ahead of it already.

@quii
Copy link
Owner

quii commented Feb 2, 2024

Nice one!

@quii quii merged commit 5fa95d6 into quii:main Feb 2, 2024
1 check passed
@drademann
Copy link
Contributor Author

I'm glad you like it. I'm still learning Go and I really enjoy your book. Great work, thank you.

fletchnj added a commit to fletchnj/learn-go-with-tests that referenced this pull request Feb 21, 2024
Simplified ConvertToArabic algorithm (quii#733)
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