-
Notifications
You must be signed in to change notification settings - Fork 246
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
Add implementation to TextProcessorJDK.topVowelAndConsonant() for haiku-kata-solutions module #296 #345
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few changes!
// Find all of the alphabetic letters from the haiku, convert them to lowercase, | ||
// and count the Character values in a Map. | ||
Map<Character, Long> charCounts = this.getHaikuAsChars() | ||
.stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IntStream does not have the method .stream() so this code doesn't compile. You already have an IntStream, you just need to convert it into a stream of character objects. I would take a look at the IntStream.mapToObj(IntFunction<? extends U> mapper) function.
https://docs.oracle.com/javase/8/docs/api/java/util/stream/IntStream.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worked on the solution as per your suggestions. Thanks for the link, It was helpful in understanding the function better.
|
||
// Find the top vowel. | ||
char topVowel = entries.stream() | ||
.filter(this::isVowel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being unclear. I think you need to .getKey() to use the isVowel method. entries
is a list of map objects and isVowel only has aa char as input. So, using a lambda with .getKey on the map makes sense to me, just like how you had in the previous one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. I made the necessary adjustments and updated the code accordingly. Thanks again for catching that.
|
||
// Find the top consonant. | ||
char topConsonant = entries.stream() | ||
.filter(!this::isvowel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above; I believe you had it right in the last time. I just wanted to get you to change the previous one. Sorry for the confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to apologize; I appreciate your input and feedback!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Nice work!
Thank you! I closed it by mistake, I reopened it, sorry. |
Added the suggested changes
4577511
to
a71c40b
Compare
Thank you for your contribution @rsp2210 and thank you, @rzrobin213 for the code review! Good work. |
Fix #296