Skip to content

Implement Robot Simulator #146

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.

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
merged 1 commit into from
Jul 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"roman-numerals",
"hexadecimal",
"grade-school",
"robot-simulator",
"queen-attack",
"sublist",
"allergies",
Expand Down
4 changes: 4 additions & 0 deletions exercises/robot-simulator/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions exercises/robot-simulator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[package]
name = "robot-simulator"
version = "0.0.0"
101 changes: 101 additions & 0 deletions exercises/robot-simulator/example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#[derive(PartialEq, Debug, Copy, Clone)]
pub enum Direction {
North,
East,
South,
West,
}

impl Direction {
pub fn previous_clockwise(&self) -> Self {
match *self {
Direction::North => Direction::West,
Direction::East => Direction::North,
Direction::South => Direction::East,
Direction::West => Direction::South,
}
}

pub fn next_clockwise(&self) -> Self {
match *self {
Direction::North => Direction::East,
Direction::East => Direction::South,
Direction::South => Direction::West,
Direction::West => Direction::North,
}
}
}

#[derive(Clone, Copy)]
struct Position {
x: i32,
y: i32,
}

impl Position {
fn new(x: i32, y: i32) -> Self {
Position { x: x, y: y }
}

fn advance(&self, direction: &Direction) -> Self {
match *direction {
Direction::North => Self::new(self.x, self.y + 1),
Direction::South => Self::new(self.x, self.y - 1),
Direction::East => Self::new(self.x + 1, self.y),
Direction::West => Self::new(self.x - 1, self.y),
}
}
}

#[derive(Clone)]
pub struct Robot {
position: Position,
direction: Direction,
}

impl Robot {
pub fn new(x: i32, y: i32, d: Direction) -> Self {
Robot::build(Position::new(x, y), d)
}

fn build(position: Position, direction: Direction) -> Self {
Robot {
position: position,
direction: direction,
}
}

pub fn turn_right(&self) -> Self {
Self::build(self.position, self.direction.next_clockwise())
}

pub fn turn_left(&self) -> Self {
Self::build(self.position, self.direction.previous_clockwise())
}

pub fn advance(&self) -> Self {
Self::build(self.position.advance(&self.direction), self.direction)
Copy link

Choose a reason for hiding this comment

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

This is personal preference but I find these one liners rather noisy and think separating it out is clearer to see what's happening:

let mut next = self.clone();
next.position = next.position.advance(self.direction);
next

Or to pattern match it into components and rebuild:

let Robot { mut position, direction } = self.clone();
position = position.advance(direction);
Self::build(position, direction)

Or with functional update syntax:

Robot {
    position: self.position.advance(&self.direction),
    // makes it more clear that only the position property is changing
    .. self.clone() 
}

}

pub fn instructions(&self, instructions: &str) -> Self {
instructions.chars().fold(self.clone(),
|robot, instruction| robot.execute(instruction))
Copy link

Choose a reason for hiding this comment

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

nit: formatting of the closure is a bit unexpected, usually I see this formatted as:

instructions.chars().fold(self.clone(), |robot, instruction| {
    robot.execute(instruction)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the formatting is being done automatically by rustfmt. I try not to fight it.

Copy link

Choose a reason for hiding this comment

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

Fair enough! :)

}

pub fn position(&self) -> (i32, i32) {
(self.position.x, self.position.y)
}

pub fn direction(&self) -> &Direction {
&self.direction
}

fn execute(self, command: char) -> Self {
match command {
'R' => self.turn_right(),
'L' => self.turn_left(),
'A' => self.advance(),
_ => self,
Copy link

Choose a reason for hiding this comment

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

Should there be a test exercising 'unknown' commands? (I would expect this to panic, or propagate an error value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good suggestion for the common test suite

Copy link

Choose a reason for hiding this comment

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

The suite mentions the possibility to have an optional test for "an invalid instruction throws an error".

}
}
}
44 changes: 44 additions & 0 deletions exercises/robot-simulator/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// The code below is a stub. Just enough to satisfy the compiler.
// In order to pass the tests you can add-to or change any of this code.

#[derive(PartialEq, Debug)]
Copy link
Contributor

@jonasbb jonasbb Jun 27, 2016

Choose a reason for hiding this comment

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

Is there a reason to not derive Eq as well? Maybe, also Copy and Clone, which makes working with references easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was only adding what I needed in order to get the compiler to compile. I imagine the real solution will have more derived traits.

Copy link
Member

Choose a reason for hiding this comment

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

The takeaway I get from this is: "If students think that they can't add more traits, we should add a comment telling them they can". I don't know whether the conditional is true (I can't read minds).

Copy link
Contributor Author

@IanWhitney IanWhitney Jul 3, 2016

Choose a reason for hiding this comment

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

For my example code I ended up not needing to derive Eq or Copy. But I did need Clone. Other students may need different traits, though.

What we might want at the top of all of our stub files is a general comment to say "This file is here to get you started. Change/add/remove code as you see fit."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment added.

Copy link
Member

Choose a reason for hiding this comment

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

Seems good. It makes me wonder that if we are going to be in the habit of providing stubs, maybe this comment should be a standard that goes on top of all of them...

clearly an idea for another PR, but neverthelss worth bringing up while we are gthinking about it here

pub enum Direction {
North,
East,
South,
West,
}

pub struct Robot;

impl Robot {
#[allow(unused_variables)]
pub fn new(x: isize, y: isize, d: Direction) -> Self {
unimplemented!()
}

pub fn turn_right(self) -> Self {
unimplemented!()
}

pub fn turn_left(self) -> Self {
unimplemented!()
}

pub fn advance(self) -> Self {
unimplemented!()
}

#[allow(unused_variables)]
pub fn instructions(self, instructions: &str) -> Self {
unimplemented!()
}

pub fn position(&self) -> (isize, isize) {
unimplemented!()
}

pub fn direction(&self) -> &Direction {
Copy link

Choose a reason for hiding this comment

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

Direction should probably be Copy which would make this idiomatically -> Direction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's anything in the tests that forces Direction to be Copy. If there is, then I agree that this signature makes more sense. But if not, then I think we should stick with &T

unimplemented!()
}
}
147 changes: 147 additions & 0 deletions exercises/robot-simulator/tests/robot-simulator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
extern crate robot_simulator;

use robot_simulator::*;

#[test]
fn robots_are_created_with_position_and_direction() {
let robot = Robot::new(0, 0, Direction::North);
assert_eq!((0, 0), robot.position());
assert_eq!(&Direction::North, robot.direction());
}

#[test]
#[ignore]
fn positions_can_be_negative() {
let robot = Robot::new(-1, -1, Direction::South);
assert_eq!((-1, -1), robot.position());
assert_eq!(&Direction::South, robot.direction());
}

#[test]
#[ignore]
fn turning_right_does_not_change_position() {
let robot = Robot::new(0, 0, Direction::North).turn_right();
assert_eq!((0, 0), robot.position());
}

#[test]
#[ignore]
fn turning_right_from_north_points_the_robot_east() {
let robot = Robot::new(0, 0, Direction::North).turn_right();
assert_eq!(&Direction::East, robot.direction());
}

#[test]
#[ignore]
fn turning_right_from_east_points_the_robot_south() {
let robot = Robot::new(0, 0, Direction::East).turn_right();
assert_eq!(&Direction::South, robot.direction());
}

#[test]
#[ignore]
fn turning_right_from_south_points_the_robot_west() {
let robot = Robot::new(0, 0, Direction::South).turn_right();
assert_eq!(&Direction::West, robot.direction());
}

#[test]
#[ignore]
fn turning_right_from_west_points_the_robot_north() {
let robot = Robot::new(0, 0, Direction::West).turn_right();
assert_eq!(&Direction::North, robot.direction());
}

#[test]
#[ignore]
fn turning_left_does_not_change_position() {
let robot = Robot::new(0, 0, Direction::North).turn_left();
assert_eq!((0, 0), robot.position());
}

#[test]
#[ignore]
fn turning_left_from_north_points_the_robot_west() {
let robot = Robot::new(0, 0, Direction::North).turn_left();
assert_eq!(&Direction::West, robot.direction());
}

#[test]
#[ignore]
fn turning_left_from_west_points_the_robot_south() {
let robot = Robot::new(0, 0, Direction::West).turn_left();
assert_eq!(&Direction::South, robot.direction());
}

#[test]
#[ignore]
fn turning_left_from_south_points_the_robot_east() {
let robot = Robot::new(0, 0, Direction::South).turn_left();
assert_eq!(&Direction::East, robot.direction());
}

#[test]
#[ignore]
fn turning_left_from_east_points_the_robot_north() {
let robot = Robot::new(0, 0, Direction::East).turn_left();
assert_eq!(&Direction::North, robot.direction());
}

#[test]
#[ignore]
fn advance_does_not_change_the_direction() {
let robot = Robot::new(0, 0, Direction::North).advance();
assert_eq!(&Direction::North, robot.direction());
}

#[test]
#[ignore]
fn advance_increases_the_y_coordinate_by_one_when_facing_north() {
let robot = Robot::new(0, 0, Direction::North).advance();
assert_eq!((0, 1), robot.position());
}

#[test]
#[ignore]
fn advance_decreases_the_y_coordinate_by_one_when_facing_south() {
let robot = Robot::new(0, 0, Direction::South).advance();
assert_eq!((0, -1), robot.position());
}

#[test]
#[ignore]
fn advance_increases_the_x_coordinate_by_one_when_facing_east() {
let robot = Robot::new(0, 0, Direction::East).advance();
assert_eq!((1, 0), robot.position());
}

#[test]
#[ignore]
fn advance_decreases_the_x_coordinate_by_one_when_facing_west() {
let robot = Robot::new(0, 0, Direction::West).advance();
assert_eq!((-1, 0), robot.position());
}

#[test]
#[ignore]
fn follow_instructions_to_move_west_and_north() {
let robot = Robot::new(0, 0, Direction::North).instructions("LAAARALA");
assert_eq!((-4, 1), robot.position());
assert_eq!(&Direction::West, robot.direction());
}

#[test]
#[ignore]
fn follow_instructions_to_move_west_and_south() {
let robot = Robot::new(2, -7, Direction::East).instructions("RRAAAAALA");
assert_eq!((-3, -8), robot.position());
assert_eq!(&Direction::South, robot.direction());
}

#[test]
#[ignore]
fn follow_instructions_to_move_east_and_north() {
let robot = Robot::new(8, 4, Direction::South).instructions("LAAARRRALLLL");
assert_eq!((11, 5), robot.position());
assert_eq!(&Direction::North, robot.direction());
}