Skip to content

Queen Attack: Replace tuple with ChessPosition #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
IanWhitney merged 1 commit into from
Apr 30, 2016
Merged

Queen Attack: Replace tuple with ChessPosition #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 45 additions & 54 deletions exercises/queen-attack/example.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,78 +1,69 @@
#[derive(Debug, PartialEq)]
pub struct Queen { position: ChessPosition }

impl Queen {
pub fn new(position: (i8, i8)) -> Result<Queen, ()> {
let position = ChessPosition::new(position);
if position.valid() {
Ok(Queen { position: position })
} else {
Err(())
}
}

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()
}
pub struct Queen {
position: ChessPosition,
}
pub trait ChessPiece {
fn position(&self) -> &ChessPosition;
fn can_attack<T: ChessPiece>(&self, other: &T) -> bool;
}

fn rank(&self) -> i8 {
self.position.rank()
impl ChessPiece for Queen {
fn position(&self) -> &ChessPosition {
&self.position
}

fn file(&self) -> i8 {
self.position.file()
}

fn sum(&self) -> i8 {
self.position.sum()
fn can_attack<T: ChessPiece>(&self, piece: &T) -> bool {
self.position.horizontal_from(&piece.position()) ||
self.position.vertical_from(&piece.position()) ||
self.position.diagonal_from(&piece.position())
}
}

fn difference(&self) -> i8 {
self.position.difference()
impl Queen {
pub fn new(position: ChessPosition) -> Queen {
Queen { position: position }
}
}

#[derive(Debug, PartialEq)]
struct ChessPosition { coordinates: (i8, i8) }
pub struct ChessPosition {
pub rank: i8,
pub file: i8,
}

impl ChessPosition {
fn new(coordinates: (i8, i8)) -> ChessPosition {
ChessPosition {coordinates: coordinates}
pub fn new(rank: i8, file: i8) -> Result<ChessPosition, String> {
let position = ChessPosition {
rank: rank,
file: file,
};

if position.is_valid() {
Ok(position)
} else {
Err(String::from("Invalid Position"))
}
}

fn is_valid(&self) -> bool {
self.rank >= 0 && self.rank <= 7 && self.file >= 0 && self.file <= 7
}

fn valid(&self) -> bool {
(self.rank() >= 0 && self.rank() <= 7) &&
(self.file() >= 0 && self.file() <= 7)
fn horizontal_from(&self, other: &ChessPosition) -> bool {
self.rank == other.rank
}

fn rank(&self) -> i8 {
self.coordinates.0
fn vertical_from(&self, other: &ChessPosition) -> bool {
self.file == other.file
}

fn file(&self) -> i8 {
self.coordinates.1
fn diagonal_from(&self, other: &ChessPosition) -> bool {
self.sum() == other.sum() || self.difference() == other.difference()
}

fn sum(&self) -> i8 {
self.rank() + self.file()
self.rank + self.file
}

fn difference(&self) -> i8 {
self.rank() - self.file()
self.rank - self.file
}
}
75 changes: 40 additions & 35 deletions exercises/queen-attack/tests/queen-attack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,79 +3,84 @@ extern crate queen_attack;
use queen_attack::*;

#[test]
fn test_queen_creation_with_valid_position() {
let white_queen = Queen::new((2,4));
assert!(white_queen.is_ok());
fn chess_position_on_the_board_is_ok() {
assert!(ChessPosition::new(2, 4).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());
fn chess_position_off_the_board_is_err() {
assert!(ChessPosition::new(-1, 2).is_err());
assert!(ChessPosition::new(8, 2).is_err());
assert!(ChessPosition::new(5, -1).is_err());
assert!(ChessPosition::new(5, 8).is_err());
}

let white_queen = Queen::new((5,-1));
assert!(white_queen.is_err());
#[test]
#[ignore]
fn queen_is_created_with_a_ok_position() {
Queen::new(ChessPosition::new(2, 4).unwrap());
}

let white_queen = Queen::new((5,8));
assert!(white_queen.is_err());
#[test]
#[ignore]
#[should_panic(expected = "Invalid Position")]
Copy link
Copy Markdown
Member

@petertseng petertseng Apr 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little unsure about placing an expectation on the exact error message, but maybe I'd feel better if it were just "panic message includes this string" rather than "panic message is exactly this string". I'm just saying that because I don't want to stop anyone who wants to provide error messages such as "rank can't be negative" or something

Copy link
Copy Markdown
Contributor Author

@IanWhitney IanWhitney Apr 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not committed to it, but I thought it was a way to introduce that optional part of should_panic. I didn't know that option existed until about 20 minutes ago.

Copy link
Copy Markdown
Member

@petertseng petertseng Apr 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read https://doc.rust-lang.org/book/testing.html and it says "The test harness will make sure that the failure message contains the provided text." (emphasis mine) so that addresses what I was thinking about.

Edit: Yup, also confirmed on code:

    #[test]
    #[should_panic(expected = "foo")]
    fn it_works() {
        panic!("foo bar baz")
    }

OK, I feel fine about keeping this.

fn queen_panics_with_err_position() {
Queen::new(ChessPosition::new(-1, 4).unwrap());
}

#[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));
fn queens_that_can_not_attack() {
let white_queen = Queen::new(ChessPosition::new(2, 4).unwrap());
let black_queen = Queen::new(ChessPosition::new(6, 6).unwrap());
assert!(!white_queen.can_attack(&black_queen));
}

#[test]
#[ignore]
fn test_can_attack_on_same_rank() {
let white_queen = Queen::new((2,4)).unwrap();
let black_queen = Queen::new((2,6)).unwrap();
fn queens_on_the_same_rank_can_attack() {
let white_queen = Queen::new(ChessPosition::new(2, 4).unwrap());
let black_queen = Queen::new(ChessPosition::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)).unwrap();
let black_queen = Queen::new((3,5)).unwrap();
fn queens_on_the_same_file_can_attack() {
let white_queen = Queen::new(ChessPosition::new(4, 5).unwrap());
let black_queen = Queen::new(ChessPosition::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)).unwrap();
let black_queen = Queen::new((0,4)).unwrap();
fn queens_on_the_same_diagonal_can_attack_one() {
Copy link
Copy Markdown
Member

@petertseng petertseng Apr 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, while we are changing the test names, maybe we can take the opportunity to bikeshed. I could see these being upper_right, upper_left, etc. rather than just one, two, etc.

But I don't actually care (I recognise I'm bikeshedding), so you can pick whatever makes the most sense.

Copy link
Copy Markdown
Contributor Author

@IanWhitney IanWhitney Apr 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I couldn't think of a good name for them either.

let white_queen = Queen::new(ChessPosition::new(2, 2).unwrap());
let black_queen = Queen::new(ChessPosition::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)).unwrap();
let black_queen = Queen::new((3,1)).unwrap();
fn queens_on_the_same_diagonal_can_attack_two() {
let white_queen = Queen::new(ChessPosition::new(2, 2).unwrap());
let black_queen = Queen::new(ChessPosition::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)).unwrap();
let black_queen = Queen::new((1,1)).unwrap();
fn queens_on_the_same_diagonal_can_attack_three() {
let white_queen = Queen::new(ChessPosition::new(2, 2).unwrap());
let black_queen = Queen::new(ChessPosition::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)).unwrap();
let black_queen = Queen::new((5,5)).unwrap();
fn queens_on_the_same_diagonal_can_attack_four() {
let white_queen = Queen::new(ChessPosition::new(2, 2).unwrap());
let black_queen = Queen::new(ChessPosition::new(5, 5).unwrap());
assert!(white_queen.can_attack(&black_queen));
}