1
\$\begingroup\$

I'm working through Advent of Code 2016 in Rust. The prizes go to the swift, but I'm aiming for well-written code. This is Day 2.

To summarize the problem statement, given a newline-separated list of directions (e.g. "ULL\nRRDDD\nLURDL\nUUUUD\n"), move about on a 9-key keypad starting at 5, freely colliding with the edges. On newlines, emit the output. In this case the answer is 1985.

I am using rustc version 1.15.0-nightly (71c06a56a 2016-12-18). My code can also be found on GitHub.

My answer for Day 2 is quite similar to my answer for Day 1.

lib.rs

// Activate impl-trait syntax and disable incompatible clippy warning.
#![feature(conservative_impl_trait)]
#![allow(unknown_lints)]
#![allow(needless_lifetimes)]

// Other features
#![feature(try_from)]

mod day_02;

pub fn day_02() {
    let day_02_answer = day_02::code(include_str!("day_02_input"));
    assert_eq!(day_02_answer, Ok("279".into()));
}

day_02.rs

use std::convert::TryFrom;

type Error = String;
type Result<T> = ::std::result::Result<T, Error>;

// Keypad structure:
// 1 2 3
// 4 5 6
// 7 8 9
pub fn code(instructions: &str) -> Result<String> {
    use self::Instruction::*;

    let mut code = String::from("");
    let mut curr_key: Key = 5;
    for instruction in Instruction::try_many_from(instructions) {
        let instruction = instruction?;
        let row = (curr_key - 1) / 3;
        let col = (curr_key - 1) % 3;
        match (instruction, row, col) {
            (Up, 0, _) | (Down, 2, _) | (Left, _, 0) | (Right, _, 2) => {}
            (Up, _, _) => {
                curr_key -= 3;
            }
            (Down, _, _) => {
                curr_key += 3;
            }
            (Left, _, _) => {
                curr_key -= 1;
            }
            (Right, _, _) => {
                curr_key += 1;
            }
            (End, _, _) => {
                code.push_str(&format!("{}", curr_key));
            }
        }
    }
    Ok(code)
}

type Key = usize;

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum Instruction {
    Up,
    Down,
    Left,
    Right,
    End,
}

impl Instruction {
    fn try_many_from<'a>(s: &'a str) -> impl Iterator<Item = Result<Self>> + 'a {
        s.chars().map(Self::try_from)
    }
}

impl TryFrom<char> for Instruction {
    type Err = Error;

    fn try_from(c: char) -> Result<Self> {
        match c {
            'U' => Ok(Instruction::Up),
            'R' => Ok(Instruction::Right),
            'D' => Ok(Instruction::Down),
            'L' => Ok(Instruction::Left),
            '\n' => Ok(Instruction::End),
            invalid => Err(format!("Instruction invalid: {}", invalid)),
        }
    }
}

#[cfg(test)]
mod test {
    use super::*;
    use super::Instruction::*;

    #[test]
    fn test_parse_instruction_success() {
        let instruction = Instruction::try_from('U');
        assert_eq!(instruction, Ok(Up));
    }

    #[test]
    fn test_parse_instructions_success() {
        let instructions = Instruction::try_many_from("ULRD\nU").collect::<Result<Vec<_>>>();
        assert_eq!(instructions, Ok(vec![Up, Left, Right, Down, End, Up]));
    }

    #[test]
    fn test_parse_instruction_invalid() {
        let instruction = Instruction::try_from('x');
        assert_eq!(instruction, Err("Instruction invalid: x".into()));
    }

    #[test]
    fn test_parse_instructions_invalid() {
        let instructions = Instruction::try_many_from("ULxDy").collect::<Result<Vec<_>>>();
        assert_eq!(instructions, Err("Instruction invalid: x".into()));
    }

    #[test]
    fn test_code() {
        let code = code("ULL\nRRDDD\nLURDL\nUUUUD\n");
        assert_eq!(code, Ok("1985".into()));
    }

    #[test]
    fn test_code_advent_input() {
        let day_02_answer = code(include_str!("day_02_input"));
        assert_eq!(day_02_answer, Ok("69642".into()));
    }
}

I use Error = String instead of Error = &'static str because I want to include erroneous instructions in the error message. Is that faulty reasoning?

\$\endgroup\$
  • \$\begingroup\$ Haha, sorry you put a bounty on it as I was typing stuff up <-- embarrassed. \$\endgroup\$ – Shepmaster Dec 31 '16 at 21:52
  • \$\begingroup\$ Have you been doing the second halves of each day, after unlocking the first part? \$\endgroup\$ – Shepmaster Dec 31 '16 at 22:16
  • \$\begingroup\$ The bounty is totally deserved; the answer is great as always. I'll award it in 22 hours when it becomes available. Also, I didn't notice there were second halves! How should I work those in? I think I'll rename days 1 and 2 with "Part 1" and put up "Part 2" separately. Then for future days I'll probably put them up concurrently. \$\endgroup\$ – Matthew Piziak Dec 31 '16 at 22:44
  • \$\begingroup\$ I've been taking it as a challenge to try and rework my code so that it can solve both parts with the same code base, which seemed to be a good way of stretching the flexibility part. \$\endgroup\$ – Shepmaster Dec 31 '16 at 22:51
  • \$\begingroup\$ Oh certainly. I'm just wondering what the best way to present it on CodeReview.SX is. \$\endgroup\$ – Matthew Piziak Dec 31 '16 at 22:52
2
+50
\$\begingroup\$

lib.rs

  1. The comments on the features have dubious value; they seem to repeat what's already said, or be ultra generic ("other features").

  2. I wouldn't broadly disable all unknown lints. Too wide a net for my tastes. Clippy provides a nicer method: only enabling things when cargo clippy is running.

  3. Likewise, I wouldn't do a package-wide allow of the incorrect lints, at least not at first. Just on the particular item. Speaking of which, this would have been a good time to document why features / lints were allowed, not that they were.
#![feature(conservative_impl_trait)]
#![feature(try_from)]

mod day_02;

pub fn day_02() {
    let day_02_answer = day_02::code(include_str!("day_02_input"));
    assert_eq!(day_02_answer, Ok("279".into()));
}

day_02.rs

  1. Use String::new, not String::from("").
  2. Use write!(&mut string, ...) instead of allocating a new string with format! then re-allocating memory to copy it.
use std::convert::TryFrom;
use std::fmt::Write;

type Error = String;
type Result<T> = ::std::result::Result<T, Error>;

// Keypad structure:
// 1 2 3
// 4 5 6
// 7 8 9
pub fn code(instructions: &str) -> Result<String> {
    use self::Instruction::*;

    let mut code = String::new();
    let mut curr_key: Key = 5;
    for instruction in Instruction::try_many_from(instructions) {
        let instruction = instruction?;
        let row = (curr_key - 1) / 3;
        let col = (curr_key - 1) % 3;
        match (instruction, row, col) {
            (Up, 0, _) | (Down, 2, _) | (Left, _, 0) | (Right, _, 2) => {}
            (Up, _, _) => {
                curr_key -= 3;
            }
            (Down, _, _) => {
                curr_key += 3;
            }
            (Left, _, _) => {
                curr_key -= 1;
            }
            (Right, _, _) => {
                curr_key += 1;
            }
            (End, _, _) => {
                write!(&mut code, "{}", curr_key).expect("Unable to append to string");
            }
        }
    }
    Ok(code)
}

type Key = usize;

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
enum Instruction {
    Up,
    Down,
    Left,
    Right,
    End,
}

impl Instruction {
    #[cfg_attr(feature = "cargo-clippy", allow(needless_lifetimes))]
    fn try_many_from<'a>(s: &'a str) -> impl Iterator<Item = Result<Self>> + 'a {
        s.chars().map(Self::try_from)
    }
}

impl TryFrom<char> for Instruction {
    type Err = Error;

    fn try_from(c: char) -> Result<Self> {
        match c {
            'U' => Ok(Instruction::Up),
            'R' => Ok(Instruction::Right),
            'D' => Ok(Instruction::Down),
            'L' => Ok(Instruction::Left),
            '\n' => Ok(Instruction::End),
            invalid => Err(format!("Instruction invalid: {}", invalid)),
        }
    }
}

#[cfg(test)]
mod test {
    use super::*;
    use super::Instruction::*;

    #[test]
    fn test_parse_instruction_success() {
        let instruction = Instruction::try_from('U');
        assert_eq!(instruction, Ok(Up));
    }

    #[test]
    fn test_parse_instructions_success() {
        let instructions = Instruction::try_many_from("ULRD\nU").collect::<Result<Vec<_>>>();
        assert_eq!(instructions, Ok(vec![Up, Left, Right, Down, End, Up]));
    }

    #[test]
    fn test_parse_instruction_invalid() {
        let instruction = Instruction::try_from('x');
        assert_eq!(instruction, Err("Instruction invalid: x".into()));
    }

    #[test]
    fn test_parse_instructions_invalid() {
        let instructions = Instruction::try_many_from("ULxDy").collect::<Result<Vec<_>>>();
        assert_eq!(instructions, Err("Instruction invalid: x".into()));
    }

    #[test]
    fn test_code() {
        let code = code("ULL\nRRDDD\nLURDL\nUUUUD\n");
        assert_eq!(code, Ok("1985".into()));
    }

    #[test]
    fn test_code_advent_input() {
        let day_02_answer = code(include_str!("day_02_input"));
        assert_eq!(day_02_answer, Ok("69642".into()));
    }
}

It's an interesting decision to parse \n as an Instruction. It kind of fits well with the domain, as you could liken it to hitting the "Enter" key on the keypad. However, the choice of newline seems incidental. If they wanted a key, I would have expected it to have a dedicated letter in the input file.

Additionally, hard-coding \n ties you to a specific platform's newline convention. This code won't work with Windows-style newlines.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.