-
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
Fix issue #291 - add implementation to WordleEC.guess(String guess) #347
Conversation
…ess(String guess)
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.
Thanks for tackling one of the harder issues and congrats on coming up with a working solutions. I really like the way you thought about the problem and commented the code; I think you're on the right track. For our Eclipse Collections Katas, we want our solutions to use the rich APIs provided by the Eclipse Collections (EC) library in their solutions. I added some comments about how to transform your solution in a more EC way.
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.
Please do not commit files in the .ideas directory.
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.
Please do not commit files in the .ideas directory.
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.
Please do not commit files in the .ideas directory.
@@ -15,7 +15,7 @@ | |||
|
|||
public class WordleECTest | |||
{ | |||
// @Test // Uncomment once guess is implemented for WordleEC | |||
@Test // Uncomment once guess is implemented for WordleEC | |||
// @Tag("SOLUTION") |
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.
Uncomment the Tag for the solutions module too. We set up the project so this tag is needed for our builds to run.
return guess.toUpperCase(); | ||
} | ||
|
||
for (int i = 0; i < guess.length(); i++) { |
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.
One of the design goals of Eclipse Collections is to provide functional and fluent APIs to process and use the collections that it provides. There are many built-in methods for MutableCharBags (especially methods to help with iteration) that you can leverage instead of writing a for-loop (hint: collectWithIndex and injectIntoWithIndex). This makes the code more readable and abstracts away many of the complexities of the code. We encourage contributors of our EC katas to use these methods in their solution. So try to solve this without using a for-loop or while-loop :)
if (!guessBag.contains(hiddenChars.charAt(i))) { | ||
hiddenBag.removeOccurrences(hiddenChars.charAt(i),hiddenBag.occurrencesOf(hiddenChars.charAt(i))); | ||
} | ||
//Delete all char which is not contained by hiddenBag in guessBag |
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.
Seems like you're finding the characters that are different between the guessBag and hiddenBag and storing them into one bag. Perhaps you can just use one of the iterators built-in the CharAdapter to build a single bag to keep track of the differences. You can add to the bag when the characters are present
MutableCharBag hiddenBag = hiddenChars.toBag(); | ||
MutableCharBag guessBag = guessChars.toBag(); | ||
|
||
char[] resChars = new char[guess.length()]; |
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.
Use collections provided by Eclipse Collections!
Closing this PR, please re-open it if you are interested in working on this issue. |
Add implementation to WordleEC.guess(String guess) for wordle-kata-solutions module #291