search

exercism / prolog

Exercism exercises in Prolog.
https://exercism.org/tracks/prolog
MIT License
29 stars 39 forks source link

some extra tests for queen-attack #122

Closed IvanRossi closed 1 year ago

IvanRossi commented 3 years ago

IMHO, tests should also check for both "positions within board" and "positions are different" inside the attack predicate.

neenjaw commented 3 years ago

I like this idea, thanks for the PR 🎉💙,

I haven't checked, but could you see if these tests are already listed in the exercism/problem-specifications repo for this problem?

This is important for a few reasons, in the upcoming version of exercism, the problem introduction will come from that repo. So it is important that our introduction specifies the assumptions present in the exercise.

Also if similar tests exist, could you align them to match their input / expected output?

If there are no similar tests, I think it's reasonable to just add these.

Does that sound reasonable?

IvanRossi commented 3 years ago

I haven't checked, but could you see if these tests are already listed in the exercism/problem-specifications repo for this problem?

This is important for a few reasons, in the upcoming version of exercism, the problem introduction will come from that repo. So it is important that our introduction specifies the assumptions present in the exercise.

Also if similar tests exist, could you align them to match their input / expected output?

If there are no similar tests, I think it's reasonable to just add these.

Does that sound reasonable?

There is similar stuff but not exactly what i did. For example the "positions within board" are applied just to the "create" property and not the "attack" one, which may make sense in some context (create a queen "object" that you pass to an "attack" function. Current tests look coherent (to me) with what it is now inside exercism/problem-specifications.

My 0.02: I would rather have this test applied to the "attack" property or you might circumvent the tests (at least in prolog).

About the test for "queen positions are different" , it says within canonical-data.json comment section:

"Some languages implement tests beyond this set, such as checking, for two pieces being placed on the same position, representing the board graphically, or using standard chess notation. Those tests can be offered as extra credit"

No idea what it is sensible to do now.

ErikSchierboom commented 1 year ago

@IvanRossi I think it would be fine to add these tests, as nothing in the instructions contradicts it. Could you resolve the merge conflict?

ErikSchierboom commented 1 year ago

@IvanRossi Did you mean to close this PR?

IvanRossi commented 1 year ago

Actually yes but I did not commit in the right place. After all it has been 2 years since I made the request. ;-)