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

feat: add pig-latin exercise #812

Merged
merged 3 commits into from
Feb 25, 2024
Merged

feat: add pig-latin exercise #812

merged 3 commits into from
Feb 25, 2024

Conversation

vaeng
Copy link
Contributor

@vaeng vaeng commented Feb 20, 2024

No description provided.

@vaeng vaeng added x:module/practice-exercise Work on Practice Exercises x:rep/large Large amount of reputation labels Feb 20, 2024
Copy link
Contributor

@ahans ahans left a comment

Choose a reason for hiding this comment

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

Here are my thoughts on your implementation. It certainly passes as a "proof of concept" either way.

for (std::string start : {"a", "e", "i", "o", "u", "yt", "xr"}) {
if (word.substr(0, start.size()) == start) return word + "ay";
}
for (std::string start : {"thr", "sch", "squ", "ch", "qu", "th", "rh"}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While this list of consonant clusters is sufficient to pass the tests, those cover only a fraction of the possibilities. Words for which the current implementation would not give the correct pig latin version include "friend", "splash", "smile", "string".

That's why I said in some other comment that my implementation is more generic. Instead of explicitly listing consonant clusters, it moves characters to the back until it finds a vowel. Only qu needs special handling in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would vote to move that discussion and possible solutions to an article for the exercise.

Copy link
Contributor

@siebenschlaefer siebenschlaefer left a comment

Choose a reason for hiding this comment

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

Looks good!

Solid example solution. There might be a few small details that could be improved (e.g. passing std::string by reference to const or calling phrase.substr(start) or some of the things from @ahans' comments) but nothing that would disqualify this as an example solution or block this PR from being merged.

@vaeng vaeng merged commit dc44239 into main Feb 25, 2024
9 checks passed
@vaeng vaeng deleted the feat--add-pig-latin-exercise branch February 25, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:module/practice-exercise Work on Practice Exercises x:rep/large Large amount of reputation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants