Repository navigation
Queen-Attack should have unit test to check if queens on same square #1641
Description
Hi - I'm going through the F# track. In the Queen-Attack module I think there should be a unit test to check and see if 'canAttack' returns false if queen1 and queen2 are on the same square:
[<Fact>]
let `Queens cannot attack if on same square`` () =
let whiteQueen = (4, 4)
let blackQueen = (4, 4)
canAttack blackQueen whiteQueen |> should equal false
'canAttack' cannot assume that it will be passed two different values for queen1 and queen2. In real life two pieces cannot occupy the same square at the same time, so 'canAttack' should take into account this edge case, in my humble opinion!
Activity
iHiD commentedon Feb 17, 2020
cc @exercism/fsharp
srpeterson commentedon Feb 17, 2020
Updated md in original comment so now shows full unit test function
NobbZ commentedon Feb 17, 2020
Its currently not covered by the canonical data:
https://github.com/exercism/problem-specifications/blob/master/exercises/queen-attack/canonical-data.json
So it should be added there as well.
Though I'd not say, that the function should return
falsein this case, but instead signal an error for invalid input in the lannguages idioms.On the other hand side, that would overcomplicate the exercise… I think we should just assume for this exercise, that such an invalid game state is guaranteed to not happen due to the surrounding game implementation.
srpeterson commentedon Feb 17, 2020
I don't think it would over complicate the exercise. Having the queen1 = queen2 check (whether it signals an error or returns false) is easy to implement. I did it in my solution (even though there is no unit tests for it currently). In my opinion, it helps the developer learn to consider all edge cases.
See: https://exercism.io/tracks/fsharp/exercises/queen-attack/solutions/7e229407a0424297b5dc3d27cd6a1c9d
Just a suggestion, my F# Exercism friends!
sshine commentedon Feb 28, 2020
I've moved the issue to exercism/problem-specifications even though it currently discusses a test idea as being implementable on the F# track in particular. This is because this repo gathers more attention to such matters. Feel free to continue the discussion in the context of the F# track. :-)
petertseng commentedon Feb 28, 2020
To make this decision, observe that there are two possible interpretations for the job of the to-be-tested function:
trueorfalseas the answer. This repo should choose one of the below actions:falseis the correct answer.To decide what action to take, it is first necessary to agree on which interpretation is to be used, or to agree to disagree.
Calamari commentedon Feb 28, 2020
In the canonical data there are checks, if the positions are on the board and throwing errors if they aren't, if I am reading this correctly. So I would also think, that having two pieces standing on the same spot fail the same invariant, that it is a valid chess position. So I would opt for an error as well.
yawpitch commentedon Mar 1, 2020
Isn't this rather covered by the discussion comment at the top of the canonical data?
Personally I don't think we need to add anything more at this level, but if we're going to add a test case then I'd also say it should error, rather than return False, as it's an illegal board position, no different than any of the other illegal positions that we currently require an error for.
ErikSchierboom commentedon Mar 10, 2020
I agree with everything here. Personally, I also don't feel we need to be exhaustive and test all invalid cases, but I don't mind it too. It definitely should be an error if it is added.
srpeterson commentedon Mar 10, 2020
I agree with Erick that not all invalid cases need to be tested, especially those far right edge cases that occur once in a blue moon. But I do think obvious invalid cases should be tested. IMHP two queens occupying the came position is also an illegal position that should have a unit test. In the current Queen Attack tests, there are tests for valid queen position, and +/- rows and columns. These are also illegal positions, yet test for a bool return value. Maybe those should also return an error?
yawpitch commentedon Mar 19, 2020
@srpeterson in what language track is that? The canonical data currently has an error state expected for positions that underflow or overflow the board bounds, while boolean values are expected only for legal positions on the board.
srpeterson commentedon Mar 19, 2020
This is the F# language track
yawpitch commentedon Mar 21, 2020
Yeah, looks like the F# track diverged somewhat from the canonical data there ... it's treating all error states as False-y, where the canonical data treats them as error states.
ErikSchierboom commentedon Mar 21, 2020
Yeah, we did that because we wanted to not have error checking in this exercise. Once the new Concept Exercises are in place, we can revert this.
IsaacG commentedon Jul 29, 2026
The interest in adding more error checking to the existing tests has decreased with time. Closing this as will not fix.