dominoes: Check that last can match first; Check that output dominoes = input dominoes #140
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petertseng
commented
Jan 23, 2017
exercises/dominoes/test.ml
Outdated
| let check_chain (input: dominoe list) (chained: dominoe list) = | ||
| let assert_dominoes_match d1 d2 = | ||
| if snd d1 <> fst d2 then failwith @@ sprintf "%s and %s cannot be chained together" (print_dominoe d1) (print_dominoe d2) else () in | ||
| let consecutives = List.zip_exn (drop_1_right chained) (List.tl_exn chained) in |
There was a problem hiding this comment.
or maybe I can change this to List.zip_exn chained (List.tl_exn chained ++ [List.hd_exn chained]), so that the list of consecutives will include (List.last_exn chained, List.hd_exn chained)
There was a problem hiding this comment.
decided I might as well do that.
Otherwise, unconditionally returning `Some [(1, 2)]` unreasonably passes
many tests (7/12, all tests expecting a `Some`).
With this commit, they will fail with:
(Failure "(1,2) and (1,2) cannot be chained together")
As part of the work in #139.
Otherwise, unconditionally returning `Some [(1, 1)]` unreasonably passes
many tests (7/12, all test expecting a `Some`).
With this commit, they will fail with:
chain doesn't use the same dominoes as the input
expected: [(1,2);(1,3);(2,3)] but got: [(1,1)]
Closes #139
|
These changes make sense. |
|
Oops, I should have updated the template as well, sorry for making you have to do it in #141! I'll leave this as a reminder for myself for next time. |
|
That's cool. It was only a couple of minutes' work to fix up. |
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #139
More detail in individual commit messages.
Sidebar: It could be the case that only the last commit is necessary - is there an input and output pair for which the consecutive-dominoes check and same-dominoes check hold, but the dominoes-at-ends check doesn't hold? Can't think of one. Nevertheless, I include the commit for completeness.