[hangman] Sync tests & Update solution to match tests.toml#3109
[hangman] Sync tests & Update solution to match tests.toml#3109thibault2705 wants to merge 2 commits intoexercism:mainfrom
Conversation
|
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos. For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
| class Hangman { | ||
|
|
||
| Observable<Output> play(Observable<String> words, Observable<String> letters) { | ||
| Observable<Output> guess(String word, Observable<String> letters) { |
There was a problem hiding this comment.
I think this change is because we have removed the "consecutive games" test, which isn't in the problem specifications. To me, if we are going to change the first argument from Observable to a String, then I think we should also remove the other Observable usages. letters can become a list and the return could be Output directly. This would simplify the exercise and remove the RxJava dependency. The exercise's .docs/instructions.append.md should also be deleted if we do this.
The other alternative is to keep the "consecutive games" test case and keep using RxJava. We can have Java track specific tests.
| static Output initialState(final String secret) { | ||
| return new Output( | ||
| secret, | ||
| getGuessedWord(secret, Collections.emptySet()), | ||
| new LinkedHashSet<>(), | ||
| new LinkedHashSet<>(), | ||
| new ArrayList<>(), | ||
| Status.ON_GOING); | ||
| } | ||
|
|
||
| boolean isLetterAlreadyPlayed(final String letter) { | ||
| return guesses.contains(letter) || misses.contains(letter); | ||
| } | ||
|
|
||
| boolean isLetterInSecret(final String letter) { | ||
| return word.contains(letter); | ||
| } | ||
|
|
||
| static String getGuessedWord(String secret, Set<String> letters) { | ||
| return secret.chars() | ||
| .mapToObj(i -> String.valueOf((char) i)) | ||
| .map(c -> letters.contains(c) ? c : "_") | ||
| .collect(joining()); | ||
| } | ||
|
|
||
| static boolean isWin(String secret, Set<String> guessedLetters) { | ||
| return secret.chars() | ||
| .mapToObj(i -> String.valueOf((char) i)) | ||
| .allMatch(guessedLetters::contains); | ||
| } | ||
|
|
||
| static boolean isLoss(List<Part> parts) { | ||
| return parts.size() >= MAX_FAILURES; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "Output{" + | ||
| "secret='" + word + '\'' + | ||
| ", discovered='" + maskedWord + '\'' + | ||
| ", guess=" + guesses + | ||
| ", misses=" + misses + | ||
| ", parts=" + parts + | ||
| ", status=" + state + | ||
| ", remainingFailures=" + remainingFailures + | ||
| '}'; | ||
| } |
There was a problem hiding this comment.
Do all these methods really need to be added? I don't see them used by the tests. If they needed by the student solution, students could write them.
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
| import java.util.*; |
There was a problem hiding this comment.
Please don't use the * import.
| import java.util.*; | |
| import java.util.Collections; | |
| import java.util.List; | |
| import java.util.Set; |
2 participants
pull request
What has been done:
play(Observable<String> words, Observable<String> letters)toguess(String word, Observable<String> letters)maskedWord,state, andremainingFailuresto match canonical expectationsThis PR partially solves issues #2959.
Reviewer Resources:
Track Policies