exercism / c

Exercism exercises in C.
https://exercism.org/tracks/c
MIT License
282 stars 176 forks source link

queen-attack test that prevents incorrectly accepted answers due to unsigned type. #1011

Open edu-bm7 opened 1 week ago

edu-bm7 commented 1 week ago

Hello everyone, this is the first time I'm opening an Issue and trying to make a PR, if anything I'm doing is wrong, please let me know.

The issue:

most solutions on the queen-attack exercise uses something like abs(queen_1.row - queen_2.row) == abs(queen_1.column - queen_2.column); The problem is when we have a underflow in the first abs() and not on the second one. Leading the students forgetting to check for underflow since row and column are Unsigned Int.

There isn't a test to test this position(when both queens are on the opposite side of their initial position, but still on the same diagonal, which is a common chest position and the result should be CAN_ATTACK). I've made a simple test that checks this position and ensure the students don't commit this mistakes.

siebenschlaefer commented 1 week ago

The tests for this exercise come from the language-agnostic problem-specifications. Any changes to those tests should be discussed in the forum first where maintainers of all tracks have a chance to participate in the discussion.

Also, I have to admit I understand neither the problem nor the proposed fix.

abs(queen_1.row - queen_2.row) == abs(queen_1.column - queen_2.column) looks fine to me, especially if you consider that the operands of the subtractions get promoted to int and therefore no integer overflow can happen.

And in your new test the queens on squares {.column = 2, .row = 7 } and {.column = 6, .row = 1} are not on the same diagonal but the test expects CAN_ATTACK.

         0   1   2   3   4   5   6   7
       ┌───┬───┬───┬───┬───┬───┬───┬───┐
      0│   │   │   │   │   │   │   │   │0
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      1│   │   │   │   │   │   │ B │   │1
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      2│   │   │   │   │   │   │   │   │2
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      3│   │   │   │   │   │   │   │   │3
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      4│   │   │   │   │   │   │   │   │4
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      5│   │   │   │   │   │   │   │   │5
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      6│   │   │   │   │   │   │   │   │6
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      7│   │   │ W │   │   │   │   │   │7
       └───┴───┴───┴───┴───┴───┴───┴───┘
         0   1   2   3   4   5   6   7
edu-bm7 commented 1 week ago

{.column = 2, .row = 7 } and {.column = 6, .row = 1} that was my mistake, sorry, it was meant to be {.column = 1, .row = 7 } and {.column = 6, .row = 2}. I've tried here with the correct values and still works, after searching for why it doesn't underflow I've found out that some arithmetics operations with types smaller than int are converted to int before the operation occurs. Sorry to not test it thoroughly and bother you all with this.

         0   1   2   3   4   5   6   7
       ┌───┬───┬───┬───┬───┬───┬───┬───┐
      0│   │   │   │   │   │   │   │   │0
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      1│   │   │   │   │   │   │   │   │1
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      2│   │   │   │   │   │   │ B │   │2
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      3│   │   │   │   │   │   │   │   │3
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      4│   │   │   │   │   │   │   │   │4
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      5│   │   │   │   │   │   │   │   │5
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      6│   │   │   │   │   │   │   │   │6
       ├───┼───┼───┼───┼───┼───┼───┼───┤
      7│   │ W │   │   │   │   │   │   │7
       └───┴───┴───┴───┴───┴───┴───┴───┘
         0   1   2   3   4   5   6   7
siebenschlaefer commented 1 week ago

No worries. Sometimes even experienced C programmers trip over the usual arithmetic conversions.

Thanks for wanting to help. Happy coding!