From 9e00097e92ccd281ca3620da9af677e2cc0e3c6e Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sat, 26 Mar 2016 11:00:50 -0600 Subject: [PATCH 1/3] Implement Queen Attack There is no common set of tests for this exercise, so implementation is a bit....free form. I think there are two interesting things to look at, what tests to use, and how to use them. Which Tests I first surveyed all of the languages that implement this exercise and looked at what they tested. In order of prevalence, the five most common test types cover: 1. Can 2 queens attack each other? 2. [Can 2 pieces occupy the same space](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L26)? 3. [Comparing string representations of the chessboard](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L33) 4. [Does a chessboard start with the queens in their default positions?](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L12) 5. [Errors are raised if an invalid position is used](https://github.com/exercism/xpython/blob/b9dfe291845a31301029d947b3cf3a2ffc85b0b6/exercises/queen-attack/queen_attack_test.py#L55) Then there are some uncommon edge-case tests. Can there be more than one chessboard? Can the board be empty? Algebraic chess notation? Etc. Of the top five, I found that only numbers 1 and 5 really cover the exercise as stated -- if I place two pieces on the board, can they attack each other? So, those are the tests that I've implemented here. Implementing the other tests requires the introduction of a Chessboard object. And also a way to identify pieces (which team they belong to, how they are represented as strings, etc.) That can be interesting to do, but I find it beyond the scope of this exercise as written. At best, I think these tests should be suggested as Extra Credit. What Order I was very particular in my test order. Some languages, for example, start with the board representation tests. This requires students to write a lot of code in order to get just one test to pass. I have found that the faster I can get the first test to pass, the better. So my first test requires nothing more than a `can_attack` function that returns false. The tests then build on that, adding complexity. Then, once `can_attack` is fully implemented, I add the curveball test of invalid chess positions, which will get students thinking about bad inputs and Rust's error handling. --- config.json | 1 + exercises/queen-attack/Cargo.lock | 4 + exercises/queen-attack/Cargo.toml | 3 + exercises/queen-attack/example.rs | 48 ++++++++++ exercises/queen-attack/tests/queen-attack.rs | 93 ++++++++++++++++++++ 5 files changed, 149 insertions(+) create mode 100644 exercises/queen-attack/Cargo.lock create mode 100644 exercises/queen-attack/Cargo.toml create mode 100644 exercises/queen-attack/example.rs create mode 100644 exercises/queen-attack/tests/queen-attack.rs diff --git a/config.json b/config.json index ae0186ec6..92c8d1401 100644 --- a/config.json +++ b/config.json @@ -22,6 +22,7 @@ "grade-school", "phone-number", "hexadecimal", + "queen-attack", "beer-song", "minesweeper", "dominoes", diff --git a/exercises/queen-attack/Cargo.lock b/exercises/queen-attack/Cargo.lock new file mode 100644 index 000000000..5ce25b95d --- /dev/null +++ b/exercises/queen-attack/Cargo.lock @@ -0,0 +1,4 @@ +[root] +name = "queen-attack" +version = "0.0.0" + diff --git a/exercises/queen-attack/Cargo.toml b/exercises/queen-attack/Cargo.toml new file mode 100644 index 000000000..ac694c4cc --- /dev/null +++ b/exercises/queen-attack/Cargo.toml @@ -0,0 +1,3 @@ +[package] +name = "queen-attack" +version = "0.0.0" diff --git a/exercises/queen-attack/example.rs b/exercises/queen-attack/example.rs new file mode 100644 index 000000000..6ad70b599 --- /dev/null +++ b/exercises/queen-attack/example.rs @@ -0,0 +1,48 @@ +#[derive(Debug, PartialEq)] +pub struct Queen { + position: (i8, i8) +} + +#[derive(Debug, PartialEq)] +pub struct InvalidQueen; + +impl Queen { + pub fn new(position: (i8, i8)) -> Result { + if (position.0 < 0 || position.0 > 7) || (position.1 < 0 || position.1 >7 ) { + Err(InvalidQueen) + } else { + Ok(Queen {position: position}) + } + } +} + +pub struct Queens { + first: Queen, + second: Queen +} + +impl Queens { + pub fn new(first: Result, second: Result) -> Queens { + Queens { + first: first.unwrap(), + second: second.unwrap() + } + } + + pub fn can_attack(&self) -> bool { + self.first.position.0 == self.second.position.0 || + self.first.position.1 == self.second.position.1 || + ( + (self.first.position.0 + self.first.position.1) + == + (self.second.position.0 + self.second.position.1) + ) + || + ( + (self.first.position.0 - self.first.position.1) + == + (self.second.position.0 - self.second.position.1) + ) + + } +} diff --git a/exercises/queen-attack/tests/queen-attack.rs b/exercises/queen-attack/tests/queen-attack.rs new file mode 100644 index 000000000..5425fb86d --- /dev/null +++ b/exercises/queen-attack/tests/queen-attack.rs @@ -0,0 +1,93 @@ +extern crate queen_attack; + +use queen_attack::*; + +#[test] +fn test_can_not_attack() { + let white_queen = Queen::new((2,4)); + let black_queen = Queen::new((6,6)); + let queens = Queens::new(white_queen, black_queen); + assert_eq!(false, queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_same_rank() { + let white_queen = Queen::new((2,4)); + let black_queen = Queen::new((2,6)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_same_file() { + let white_queen = Queen::new((4,5)); + let black_queen = Queen::new((3,5)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_first_diagonal() { + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((0,4)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_second_diagonal() { + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((3,1)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_third_diagonal() { + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((1,1)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_can_attack_on_fourth_diagonal() { + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((5,5)); + let queens = Queens::new(white_queen, black_queen); + assert!(queens.can_attack()); +} + +#[test] +#[ignore] +fn test_queen_with_invalid_position() { + let white_queen = Queen::new((-1,2)); + match white_queen { + Ok(_) => panic!("This queen should be invalid"), + Err(_) => assert!(true) + } + + let white_queen = Queen::new((8,2)); + match white_queen { + Ok(_) => panic!("This queen should be invalid"), + Err(_) => assert!(true) + } + + let white_queen = Queen::new((2,-1)); + match white_queen { + Ok(_) => panic!("This queen should be invalid"), + Err(_) => assert!(true) + } + + let white_queen = Queen::new((2,8)); + match white_queen { + Ok(_) => panic!("This queen should be invalid"), + Err(_) => assert!(true) + } +} From 2a74be0b50f803e0e7ac4514c44e7d1f16a0eb0f Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Sun, 27 Mar 2016 12:49:28 -0600 Subject: [PATCH 2/3] Refactor to simpler interface As discussed here: http://doc.rust-lang.org/std/result/enum.Result.html#variant.Err I like the `queen.can_attack(queen2)` API a lot better. It has a downside of `Queen::new` returning a Result, so all tests that check for valid attacks have to call `unwrap` Though that might just be because there's a better way to do this that I am unaware of. I'm still quite new to Rust, especially the error-handling part of it. The us of `is_err` was recommended by Manishearth in this thread https://users.rust-lang.org/t/what-am-i-not-understanding-about-result/5143/11?u=ianwhitney --- exercises/queen-attack/example.rs | 62 +++++++++----------- exercises/queen-attack/tests/queen-attack.rs | 49 ++++++---------- 2 files changed, 44 insertions(+), 67 deletions(-) diff --git a/exercises/queen-attack/example.rs b/exercises/queen-attack/example.rs index 6ad70b599..c7981ec41 100644 --- a/exercises/queen-attack/example.rs +++ b/exercises/queen-attack/example.rs @@ -1,48 +1,40 @@ #[derive(Debug, PartialEq)] -pub struct Queen { - position: (i8, i8) -} - -#[derive(Debug, PartialEq)] -pub struct InvalidQueen; +pub struct Queen { position: (i8, i8) } impl Queen { - pub fn new(position: (i8, i8)) -> Result { - if (position.0 < 0 || position.0 > 7) || (position.1 < 0 || position.1 >7 ) { - Err(InvalidQueen) + pub fn new(position: (i8, i8)) -> Queen { + Queen { position: position } + } + + pub fn can_attack(&self, piece: Queen) -> Result { + if self.position_valid() && piece.position_valid() { + Ok(self.rank() == piece.rank() || + self.file() == piece.file() || + self.sum() == piece.sum() || + self.difference() == piece.difference()) } else { - Ok(Queen {position: position}) + Err(()) } } -} -pub struct Queens { - first: Queen, - second: Queen -} + fn position_valid(&self) -> bool { + (self.rank() >= 0 && self.rank() <= 7) && + (self.file() >= 0 && self.file() <= 7) + } -impl Queens { - pub fn new(first: Result, second: Result) -> Queens { - Queens { - first: first.unwrap(), - second: second.unwrap() - } + fn rank(&self) -> i8 { + self.position.0 } - pub fn can_attack(&self) -> bool { - self.first.position.0 == self.second.position.0 || - self.first.position.1 == self.second.position.1 || - ( - (self.first.position.0 + self.first.position.1) - == - (self.second.position.0 + self.second.position.1) - ) - || - ( - (self.first.position.0 - self.first.position.1) - == - (self.second.position.0 - self.second.position.1) - ) + fn file(&self) -> i8 { + self.position.1 + } + + fn sum(&self) -> i8 { + self.rank() + self.file() + } + fn difference(&self) -> i8 { + self.rank() - self.file() } } diff --git a/exercises/queen-attack/tests/queen-attack.rs b/exercises/queen-attack/tests/queen-attack.rs index 5425fb86d..ad7c485a7 100644 --- a/exercises/queen-attack/tests/queen-attack.rs +++ b/exercises/queen-attack/tests/queen-attack.rs @@ -6,8 +6,7 @@ use queen_attack::*; fn test_can_not_attack() { let white_queen = Queen::new((2,4)); let black_queen = Queen::new((6,6)); - let queens = Queens::new(white_queen, black_queen); - assert_eq!(false, queens.can_attack()); + assert_eq!(false, white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -15,8 +14,7 @@ fn test_can_not_attack() { fn test_can_attack_on_same_rank() { let white_queen = Queen::new((2,4)); let black_queen = Queen::new((2,6)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -24,8 +22,7 @@ fn test_can_attack_on_same_rank() { fn test_can_attack_on_same_file() { let white_queen = Queen::new((4,5)); let black_queen = Queen::new((3,5)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -33,8 +30,7 @@ fn test_can_attack_on_same_file() { fn test_can_attack_on_first_diagonal() { let white_queen = Queen::new((2,2)); let black_queen = Queen::new((0,4)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -42,8 +38,7 @@ fn test_can_attack_on_first_diagonal() { fn test_can_attack_on_second_diagonal() { let white_queen = Queen::new((2,2)); let black_queen = Queen::new((3,1)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -51,8 +46,7 @@ fn test_can_attack_on_second_diagonal() { fn test_can_attack_on_third_diagonal() { let white_queen = Queen::new((2,2)); let black_queen = Queen::new((1,1)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] @@ -60,34 +54,25 @@ fn test_can_attack_on_third_diagonal() { fn test_can_attack_on_fourth_diagonal() { let white_queen = Queen::new((2,2)); let black_queen = Queen::new((5,5)); - let queens = Queens::new(white_queen, black_queen); - assert!(queens.can_attack()); + assert!(white_queen.can_attack(black_queen).unwrap()); } #[test] #[ignore] fn test_queen_with_invalid_position() { let white_queen = Queen::new((-1,2)); - match white_queen { - Ok(_) => panic!("This queen should be invalid"), - Err(_) => assert!(true) - } + let black_queen = Queen::new((5,5)); + assert!(white_queen.can_attack(black_queen).is_err()); let white_queen = Queen::new((8,2)); - match white_queen { - Ok(_) => panic!("This queen should be invalid"), - Err(_) => assert!(true) - } + let black_queen = Queen::new((5,5)); + assert!(white_queen.can_attack(black_queen).is_err()); - let white_queen = Queen::new((2,-1)); - match white_queen { - Ok(_) => panic!("This queen should be invalid"), - Err(_) => assert!(true) - } + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((5,-1)); + assert!(white_queen.can_attack(black_queen).is_err()); - let white_queen = Queen::new((2,8)); - match white_queen { - Ok(_) => panic!("This queen should be invalid"), - Err(_) => assert!(true) - } + let white_queen = Queen::new((2,2)); + let black_queen = Queen::new((5,8)); + assert!(white_queen.can_attack(black_queen).is_err()); } From b452b25fc3ec79c0e574927369c2adef63f2fafa Mon Sep 17 00:00:00 2001 From: Ian Whitney Date: Mon, 28 Mar 2016 13:58:25 -0600 Subject: [PATCH 3/3] Move Result return to Queen::new Prompted by the discussion with @petertseng here: https://github.com/exercism/xrust/pull/89#issuecomment-202289566 The standard tests show that Queen creation should throw an error, not the `can_attack` method. To follow that, I've switched `Queen::new` to return a `Result`. This has required a change to the tests, as each `Queen::new` now has to be unwrapped. I've reordered the tests to make that less weird (hopefully). The first test now exercises the creation of a Queen with a valid position, using `is_ok()` which should points students directly at the Result return type. We then test the invalid queen creation. Then we can move on to `can_attack`. And I've updated the Example implementation to match this new API, while also polishing it a bit. --- exercises/queen-attack/example.rs | 66 +++++++++++---- exercises/queen-attack/tests/queen-attack.rs | 85 ++++++++++---------- 2 files changed, 96 insertions(+), 55 deletions(-) diff --git a/exercises/queen-attack/example.rs b/exercises/queen-attack/example.rs index c7981ec41..6458c48df 100644 --- a/exercises/queen-attack/example.rs +++ b/exercises/queen-attack/example.rs @@ -1,33 +1,71 @@ #[derive(Debug, PartialEq)] -pub struct Queen { position: (i8, i8) } +pub struct Queen { position: ChessPosition } impl Queen { - pub fn new(position: (i8, i8)) -> Queen { - Queen { position: position } - } - - pub fn can_attack(&self, piece: Queen) -> Result { - if self.position_valid() && piece.position_valid() { - Ok(self.rank() == piece.rank() || - self.file() == piece.file() || - self.sum() == piece.sum() || - self.difference() == piece.difference()) + pub fn new(position: (i8, i8)) -> Result { + let position = ChessPosition::new(position); + if position.valid() { + Ok(Queen { position: position }) } else { Err(()) } } - fn position_valid(&self) -> bool { + pub fn can_attack(&self, piece: Queen) -> bool { + self.horizontal_from(&piece) || + self.vertical_from(&piece) || + self.diagonal_from(&piece) + } + + fn horizontal_from(&self, piece: &Queen) -> bool { + self.rank() == piece.rank() + } + + fn vertical_from(&self, piece: &Queen) -> bool { + self.file() == piece.file() + } + + fn diagonal_from(&self, piece: &Queen) -> bool { + self.sum() == piece.sum() || + self.difference() == piece.difference() + } + + fn rank(&self) -> i8 { + self.position.rank() + } + + fn file(&self) -> i8 { + self.position.file() + } + + fn sum(&self) -> i8 { + self.position.sum() + } + + fn difference(&self) -> i8 { + self.position.difference() + } +} + +#[derive(Debug, PartialEq)] +struct ChessPosition { coordinates: (i8, i8) } + +impl ChessPosition { + fn new(coordinates: (i8, i8)) -> ChessPosition { + ChessPosition {coordinates: coordinates} + } + + fn valid(&self) -> bool { (self.rank() >= 0 && self.rank() <= 7) && (self.file() >= 0 && self.file() <= 7) } fn rank(&self) -> i8 { - self.position.0 + self.coordinates.0 } fn file(&self) -> i8 { - self.position.1 + self.coordinates.1 } fn sum(&self) -> i8 { diff --git a/exercises/queen-attack/tests/queen-attack.rs b/exercises/queen-attack/tests/queen-attack.rs index ad7c485a7..cbe061dea 100644 --- a/exercises/queen-attack/tests/queen-attack.rs +++ b/exercises/queen-attack/tests/queen-attack.rs @@ -3,76 +3,79 @@ extern crate queen_attack; use queen_attack::*; #[test] -fn test_can_not_attack() { +fn test_queen_creation_with_valid_position() { let white_queen = Queen::new((2,4)); - let black_queen = Queen::new((6,6)); - assert_eq!(false, white_queen.can_attack(black_queen).unwrap()); + assert!(white_queen.is_ok()); +} + +#[test] +#[ignore] +fn test_queen_creation_with_incorrect_positions() { + let white_queen = Queen::new((-1,2)); + assert!(white_queen.is_err()); + + let white_queen = Queen::new((8,2)); + assert!(white_queen.is_err()); + + let white_queen = Queen::new((5,-1)); + assert!(white_queen.is_err()); + + let white_queen = Queen::new((5,8)); + assert!(white_queen.is_err()); +} + +#[test] +#[ignore] +fn test_can_not_attack() { + let white_queen = Queen::new((2,4)).unwrap(); + let black_queen = Queen::new((6,6)).unwrap(); + assert_eq!(false, white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_same_rank() { - let white_queen = Queen::new((2,4)); - let black_queen = Queen::new((2,6)); - assert!(white_queen.can_attack(black_queen).unwrap()); + let white_queen = Queen::new((2,4)).unwrap(); + let black_queen = Queen::new((2,6)).unwrap(); + assert!(white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_same_file() { - let white_queen = Queen::new((4,5)); - let black_queen = Queen::new((3,5)); - assert!(white_queen.can_attack(black_queen).unwrap()); + let white_queen = Queen::new((4,5)).unwrap(); + let black_queen = Queen::new((3,5)).unwrap(); + assert!(white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_first_diagonal() { - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((0,4)); - assert!(white_queen.can_attack(black_queen).unwrap()); + let white_queen = Queen::new((2,2)).unwrap(); + let black_queen = Queen::new((0,4)).unwrap(); + assert!(white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_second_diagonal() { - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((3,1)); - assert!(white_queen.can_attack(black_queen).unwrap()); + let white_queen = Queen::new((2,2)).unwrap(); + let black_queen = Queen::new((3,1)).unwrap(); + assert!(white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_third_diagonal() { - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((1,1)); - assert!(white_queen.can_attack(black_queen).unwrap()); + let white_queen = Queen::new((2,2)).unwrap(); + let black_queen = Queen::new((1,1)).unwrap(); + assert!(white_queen.can_attack(black_queen)); } #[test] #[ignore] fn test_can_attack_on_fourth_diagonal() { - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((5,5)); - assert!(white_queen.can_attack(black_queen).unwrap()); -} - -#[test] -#[ignore] -fn test_queen_with_invalid_position() { - let white_queen = Queen::new((-1,2)); - let black_queen = Queen::new((5,5)); - assert!(white_queen.can_attack(black_queen).is_err()); - - let white_queen = Queen::new((8,2)); - let black_queen = Queen::new((5,5)); - assert!(white_queen.can_attack(black_queen).is_err()); - - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((5,-1)); - assert!(white_queen.can_attack(black_queen).is_err()); - - let white_queen = Queen::new((2,2)); - let black_queen = Queen::new((5,8)); - assert!(white_queen.can_attack(black_queen).is_err()); + let white_queen = Queen::new((2,2)).unwrap(); + let black_queen = Queen::new((5,5)).unwrap(); + assert!(white_queen.can_attack(black_queen)); }