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

Acronym: New test case for apostrophe #1377

Merged
merged 3 commits into from
Nov 5, 2018
Merged

Conversation

alanvardy
Copy link
Contributor

@alanvardy alanvardy commented Oct 23, 2018

This is a redo of PR #1375, to be held until later review!

The current solution for Acronym doesn't account for apostrophes, it is a more rare case for acronyms but an edge case nonetheless. Is this useful?

I added a single test case and incremented the version number.

"description": "apostrophes",
"property": "abbreviate",
"input": {
"phrase": "Haley's Comet"
Copy link
Member

Choose a reason for hiding this comment

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

If I may -

If this is to be representative of a real-world phrase, then I would suggest using Halley's Comet here with the extra L.

However, if all resemblances to real-world phenomena are purely coincidental, then as you were.

@rpottsoh
Copy link
Member

Thanks for fixing spelling mistake. Good eyes @petertseng.

@alanvardy
Copy link
Contributor Author

alanvardy commented Oct 29, 2018

Wow, I'm shocked that I missed that. The website I originally looked it up on misspelled Halley's and I never checked any other sources. Thanks @petertseng

@rpottsoh
Copy link
Member

rpottsoh commented Nov 1, 2018

@F3PiX let me know when it is OK to merge this. Recall you originally wanted to hold off due to some organizational changes taking place in the Ruby track.

@rpottsoh
Copy link
Member

rpottsoh commented Nov 1, 2018

I would just like to add that some students like to use regex, like this [A-Z]+[a-z]*|[a-z]+ for example to "find the words" in the input phrase. Apostrophes trip'em up.

@emcoding
Copy link
Contributor

emcoding commented Nov 1, 2018

I'm afraid I was confusing the Ruby tests with the canonical data, so the Ruby restructuring has nothing to do with merging this. Sorry for the confusing :-)

some students like to use regex, like this [A-Z]+[a-z]*|[a-z]+ for example to "find the words"

That's already taken care of by the test in line 44, with the hyphen as a non-space, non-word character?

@rpottsoh
Copy link
Member

rpottsoh commented Nov 1, 2018

That's already taken care of by the test in line 44, with the hyphen as a non-space, non-word character?

If you apply that regex to Halley's Comet it results in Halley s Comet. Some students solutions then return HSC as the acronym instead of the correct acronym of HC.

With the input of Complementary metal-oxide semiconductor or CMOS, the hyphen is separating two words so can be easily ignored without much thought.

@rpottsoh
Copy link
Member

rpottsoh commented Nov 1, 2018

What would the acronym be for O'Malley's Flux Recombinator? OMFR or OFR?

@coriolinus
Copy link
Member

coriolinus commented Nov 2, 2018 via email

@rpottsoh
Copy link
Member

rpottsoh commented Nov 2, 2018

That's a bit advanced for this exercise, though; I'd recommend against

I agree. It came to mind while I was thinking about this exercise and I found it interesting.

@rpottsoh
Copy link
Member

rpottsoh commented Nov 4, 2018

As long as there are no objections or is no further discussion I will likely merge this PR tomorrow (Sunday). If someone else decides to merge this in the mean time, please squash while merging.

@rpottsoh rpottsoh merged commit 2e46654 into exercism:master Nov 5, 2018
@rpottsoh
Copy link
Member

rpottsoh commented Nov 5, 2018

Thanks for working on this @alanvardy. 👍

@alanvardy
Copy link
Contributor Author

Thank you!

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.

5 participants