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

Kotlin / Bob / DigDeeper: Add a pattern matching approach using when-expression #629

Merged
merged 6 commits into from
Apr 9, 2024

Conversation

micha-b
Copy link
Contributor

@micha-b micha-b commented Apr 1, 2024

Hey there,

as proposed in the forum here I added a new approach to the Bob exercise on the Kotlin track using a when-expression.

I hope I provided all the files and changes needed for a correctly working version.

I am not quite sure if the entry in the config.json file needs to be done manually (as I did) or if this entry is somehow generated automatically. (I used a randomly generated uuid for the entry.)

Please have a look at the proposed changes and give feedback on the writing and configuration.

Have a nice day! 👋🏼

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

I am not quite sure if the entry in the config.json file needs to be done manually (as I did) or if this entry is somehow generated automatically. (I used a randomly generated uuid for the entry.)

Both are fine. You could generate it via:

bin/fetch-configlet
bin/configlet create --approach when-expression --exercise bob

Copy link
Member

Choose a reason for hiding this comment

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

The snippet can be at most 8 lines. You should try to pick the lines that best represent the approach (so that would be the when expression, with the function definition I think).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The length of the snippet including the function definition works out to exactly eight lines. 👍🏼

Comment on lines 16 to 23
private fun String.isYelling(): Boolean {
val letters = this.filter { it.isLetter() }
return if (letters.isEmpty())
false
else
letters.all { it.isUpperCase() }
}
}
Copy link
Member

Choose a reason for hiding this comment

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

What about?

Suggested change
private fun String.isYelling(): Boolean {
val letters = this.filter { it.isLetter() }
return if (letters.isEmpty())
false
else
letters.all { it.isUpperCase() }
}
}
private fun String.isYelling() = any(Char::isLetter) && toUpperCase() == this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey,
looks good. Tried a solution using higher order functions but did not find an approach I was happy with.

I will update the solution using this one-liner, thanks.

Comment on lines +14 to +15
private fun String.isSilence(): Boolean = this.isBlank()
private fun String.isQuestion(): Boolean = this.trim().endsWith('?')
Copy link
Member

Choose a reason for hiding this comment

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

The return types could be omitted (feel free to ignore this suggestion)

Suggested change
private fun String.isSilence(): Boolean = this.isBlank()
private fun String.isQuestion(): Boolean = this.trim().endsWith('?')
private fun String.isSilence() = this.isBlank()
private fun String.isQuestion() = this.trim().endsWith('?')

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 usually really like explicit return types because I think it makes the intent and outcome of a function easier to read.

But I know that you are able to omit them in Kotlin if you want to.
But I usually don't omit them.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, that's perfectly fine :)

@micha-b
Copy link
Contributor Author

micha-b commented Apr 9, 2024

I updated the code using your suggestions.
You can take a look at the updated code. :)

Sorry for the rather long delay.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good.

@ErikSchierboom ErikSchierboom merged commit 8e81747 into exercism:main Apr 9, 2024
2 checks passed
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