Two of the tests in Queen Attack check the same diagonal.
Specifically, tests for diagonals “three” and “four” are the same direction.
Suggestion: change test for diagonal “three” to use (2, 2) and (3, 3) instead of (2, 2) and (1, 1).
As a check, I was able to make all tests pass even with only three of the four diagonals covered by my code.
Making the suggested change to the test requires the fourth check to make them all pass.
That sounds like a good catch.
Could you show us the code that incorrectly passes with the current test cases and no longer passes with the new test case? That will make it extremely easy to approve a PR on the problem-specs repository which will propagate this change to all tracks that implement it.
In the canonical tests, I think is is “can attack on first diagonal” and “can attack on second diagonal” that check the same diagonal.
Diagonals “third” and “fourth” are parallel but distinct.
I hope a published iteration is a good demonstration: gjbianco's solution for Queen Attack in C on Exercism
In particular, note the commented lines 22-23.
If I change the test case (locally) to my suggestion, the tests no longer pass without uncommenting those lines.
EDIT: once given the go-ahead, I’m more than happy to open a PR.
I have double-checked the values in the canonical tests that you linked and it still seems like tests three and four are the duplicates.
If I arrange the grid as 0-7 rows with 0-7 columns, top-down and left-to-right respectively, “third diagonal” checks the northwest diagonal and so does “fourth diagonal”.
May I bump this thread? Stumbled upon the very same problem with the Rust version of the exercise right now. @gjbianco was totally right, test cases for the third and fourth diagonals check the same northwest direction. It’s quite puzzling for a student. :) Should I open a PR?
Two of the tests in Queen Attack check the same diagonal.
Specifically, tests for diagonals “three” and “four” are the same direction.
Tests “can attack on first diagonal” and “can attack on second diagonal” are in opposite directions on the same diagonal: the diagonal containing { (0,4) (1,3) (2,2) (3,1) (4,0) }
Tests “can attack on third diagonal” and “can attack on fourth diagonal” are in the same direction on distinct diagonals: one is on the main diagonal { (0,0) (1,1) (2,2) (3,3) (4,4) (5,5) (6,6) (7,7) } and the other is the diagonal { (0,6) (1,7) }
With any PR, we should have 4 distinct diagonals, not just 4 distinct directions on less than 4 distinct diagonals, as the test names say “first diagonal”, “second diagonal”, etc.
Is the issue the fact that the test name implies “second diagonal” can be interpreted as a different direction vs a different test than the first diagonal? You can also read second diagonal as the second test for a diagonal value.
The test names aren’t really supposed to be something students are paying much attention to. The tests in general aim to guide the implementation and are not meant to cover every case. I’m not convinced these tests need to be reimplemented.
I agree that the names aren’t much of a problem here, though I see the point that @keiraville is trying to make. It would probably be more correct to call these not “diagonals”, but “directions”. At this point the nitpicker in me also wants to put the tests in clockwise order: northeast, southeast, southwest, northwest. :)
Still, it looks like a minor issue that doesn’t affect the implementation of the exercise, unlike the thread subject.