[#768] New practice exercise go-counting
#776
Conversation
|
Thank you for contributing to Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:
Automated comment created by PR Commenter 🤖. |
| "practices": [ | ||
| "list-comprehensions" | ||
| ], | ||
| "difficulty": 9 |
There was a problem hiding this comment.
It took me a while to solve, so I put a high difficulty, but maybe I overdid it?
Anything go-related probably deserves a 9 😬
| assert Enum.sort(w1) == Enum.sort(w2) | ||
| end | ||
|
|
||
| def equal?(_a, _b), do: flunk("Unexpected output") |
There was a problem hiding this comment.
This is the current error message:
1) test Black corner territory on 5x5 board (GoCountingTest)
test/go_counting_test.exs:18
Unexpected output
code: equal?(output, expected)
stacktrace:
test/go_counting_test.exs:25: (test)
Not great because I have no idea what my function actually returned 🤔
If we fall back to a simple assertion:
| def equal?(_a, _b), do: flunk("Unexpected output") | |
| def equal?(a, b), do: assert a == b |
The error will be much nicer:
1) test Black corner territory on 5x5 board (GoCountingTest)
test/go_counting_test.exs:18
Assertion with == failed
code: assert a == b
left: nil
right: {:ok, %{owner: :black, territory: [{0, 0}, {0, 1}, {1, 0}]}}
stacktrace:
test/go_counting_test.exs:25: (test)
This case will be triggered when somebody for example misspells the error string ({:error, "Invalid coordinates"} instead {:error, "Invalid coordinate"}), so it's important that it shows all the details.
Edit: this won't matter if we remove equal? altogether
| assert Enum.sort(b1) == Enum.sort(b2) | ||
| assert Enum.sort(n1) == Enum.sort(n2) | ||
| assert Enum.sort(w1) == Enum.sort(w2) |
There was a problem hiding this comment.
I think I would prefer if we assumed that the coordinates need to be already sorted (by first coordinate first, then second coordinate) and removed the custom equal? assertions altogether. In the canonical data, they are sorted and I don't see a note anything about allowing not sorting 🤔.
We have that assumption in at least one other exercise already, saddle-points.
There was a problem hiding this comment.
OK, let's assume coordinates are sorted, it makes everything cleaner. Although I had to modify my example code :)
|
|
||
| # @tag :pending | ||
| test "Black corner territory on 5x5 board" do | ||
| board = [" B ", " B B ", "B W B", " W W ", " W "] |
There was a problem hiding this comment.
Let's do multiline strings as input. It will be easier to read and not that much harder to implement a solution. In problem specifications, sometimes there is a comment that a list of strings is supposed to be a multiline string (for example: https://github.com/exercism/problem-specifications/blob/e2779fc84ff07cb5bd8ff89f38e78af6d17b5056/exercises/proverb/canonical-data.json#L4-L6). It's weird that it's missing for this exercise.
We will need this hack to keep trailing whitespace in a multiline string:
elixir/exercises/practice/diamond/test/diamond_test.exs
Lines 53 to 106 in 9565a2f
Or maybe it won't be that much easier to read, in that case we should at least split the list items each in a new line, so that it kinda reads like a multiline string while still being a list 🙂
There was a problem hiding this comment.
Very good suggestion, thank you.
It was a bit awkward to deal with the leading/trailing white spaces, so instead I used "_" for empty spaces.
There was a problem hiding this comment.
it looks great with "_", good idea!
| @type position :: {integer, integer} | ||
| @type owner :: %{owner: atom, territory: [position]} | ||
| @type territories :: %{white: [position], black: [position], none: [position]} |
angelikatyborska
added
x:size/large
Co-authored-by: Angelika Tyborska <[email protected]>
Another one. Again, please take your time for the review, I'm just enjoying a week off.
It took me a while to solve, so I put a high difficulty, but maybe I overdid it?