3
\$\begingroup\$

I'm trying to write idiomatic and fast Rust code by solving AoC challenges this year! :-)

Link to AoC Day 2 problem

The input format in this problem is a list of lines each containing one string token (one of forward/down/up) and one integer, separated by whitespace. The number of lines is not known in advance. Example:

forward 5
down 5
forward 8
up 3
down 8
forward 2

This is my Rust code for taking input:

use std::{
    fs::File,
    io::{BufRead, BufReader, Result as io_result},
};

enum Action {
    F,
    D,
    U,
}

fn read_inputs() -> io_result<Vec<(Action, u32)>> {
    let input_file = File::open("inputs/2.txt")?;
    let file_reader = BufReader::new(input_file);
    let inputs = file_reader
        .lines()
        .map(|line| {
            let line_res = line.unwrap();
            let mut res = line_res.split(" ");
            let (action, value) = (res.next().unwrap(), res.next().unwrap());
            return {
                (
                    {
                        if action == "forward" {
                            Action::F
                        } else {
                            if action == "down" {
                                Action::D
                            } else {
                                Action::U
                            }
                        }
                    },
                    value.parse().unwrap(),
                )
            };
        })
        .collect::<Vec<(Action, u32)>>();
    return Ok(inputs);
}

I'm unsure about my code for a number of reasons:

  1. Four unwraps in a single closure. If we could reduce the unwraps on res.next(), perhaps by specifying the line is guaranteed to contain a tuple of two tokens, it would be great.
  2. The nesting depth of the ternary if-else is exceedingly high, not sure how to better it though.
  3. Perhaps if this was a very large file, would using .lines() still be reasonable or could we make this code faster? (preferably relying on Rust's zero cost abstractions)

Looking forward to advise on this code along these points and any other point as you deem fit.


The part of my code which solves the actual problem, I'm pretty happy with that honestly because I got to use a match clause :-) Still if there's anything that could be improved here, in terms of idiomatic or faster Rust, please do advise!

fn part1(inputs: &Vec<(Action, u32)>) -> u32 {
    let (mut horizontal, mut depth): (u32, u32) = (0, 0);

    for (action, movement) in inputs {
        match action {
            Action::F => {
                horizontal += movement;
            }
            Action::D => {
                depth += movement;
            }
            Action::U => {
                depth -= movement;
            }
        }
    }

    return horizontal * depth;
}

fn main() {
    let inputs = read_inputs().expect("Input read correctly");
    let answer = part1(&inputs);
    println!("Answer: {}", answer)
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  1. Let's address this
  2. Let's address this too
  3. I looked up the definition of io::Lines, its overhead is one heap allocation per line, so the cost is small but real. buf.read_line with a reused String would be slightly faster.

    let inputs = file_reader
        .lines()
        .map(|line| {
            let line_res = line.unwrap();
            // what if the line is empty?
            let mut res = line_res.split(" ");
            // multiple unwraps, kill 'em all with one match in functional style
            let (action, value) = (res.next().unwrap(), res.next().unwrap());
            return {
                (
                    {
                        // let's factor out this logic, makes sense to put it in another self-contained function
                        // because it's a piece of logic related only to Action
                        if action == "forward" {
                            Action::F
                        } else {
                            if action == "down" {
                                Action::D
                            } else {
                                Action::U
                            }
                        }
                    },
                    value.parse().unwrap(),
                )
            };
        })

// kill all unwraps. Check three items for good measure.
match (res.next(), res.next(), res.next()) { /* ... */ }

// factored out our action parsing logic
impl FromStr for Action {
    type Err = ();

    fn from_str(input: &str) -> Result<Action, Self::Err> {
        match input {
            "forward" => Ok(Action::F),
            "down" => Ok(Action::D),
            "up" => Ok(Action::U),
            _ => Err(()),
        }
    }
}
// new import
use std::{
    /* ... */
    str::FromStr,
};

            // ignore empty line
            // helps with empty lines and files ending with newline
            if line_res.is_empty() { return None; }
        // changing our `map` into `filter_map` for ignoring empty lines
        .lines()
        .filter_map(|line| {

            // let's match on correct input and fail otherwise
            match (res.next(), res.next(), res.next()) {
                // meaningful line
                (Some(action), Some(movement), None) => { ... }
                // incorrect input
                _ => panic!("incorrect input line: '{}'", line_res),
            }

            // prepare our functions up front in functional style
            // parsing
            let parse_action = Action::from_str;
            let parse_movement = |movement: &str| movement.parse();
            // processing
            match (res.next().map(parse_action), res.next().map(parse_movement), res.next()) {
                // meaningful line
                (Some(Ok(action)), Some(Ok(movement)), None) => {
                    Some((action, movement))
                }
                // incorrect input
                _ => panic!("incorrect input line: '{}'", line_res),
            }

Full code

use std::{
    fs::File,
    io::{BufRead, BufReader, Result as io_result},
    str::FromStr,
};

enum Action {
    F,
    D,
    U,
}

impl FromStr for Action {
    type Err = ();

    fn from_str(input: &str) -> Result<Action, Self::Err> {
        match input {
            "forward" => Ok(Action::F),
            "down" => Ok(Action::D),
            "up" => Ok(Action::U),
            _ => Err(()),
        }
    }
}

fn read_inputs() -> io_result<Vec<(Action, u32)>> {
    let input_file = File::open("inputs/2.txt")?;
    let file_reader = BufReader::new(input_file);
    let inputs = file_reader
        .lines()
        .filter_map(|line| {
            let line_res = line.ok()?;
            // empty line
            if line_res.is_empty() { return None; }
            let mut res = line_res.split(" ");
            // parsing
            let parse_action = Action::from_str;
            let parse_movement = |movement: &str| movement.parse();
            // processing
            match (res.next().map(parse_action), res.next().map(parse_movement), res.next()) {
                // meaningful line
                (Some(Ok(action)), Some(Ok(movement)), None) => {
                    Some((action, movement))
                }
                // incorrect input
                _ => panic!("incorrect input line: '{}'", line_res),
            }
        })
        .collect::<Vec<(Action, u32)>>();
    return Ok(inputs);
}

fn part1(inputs: &Vec<(Action, u32)>) -> u32 {
    let (mut horizontal, mut depth): (u32, u32) = (0, 0);

    for (action, movement) in inputs {
        match action {
            Action::F => {
                horizontal += movement;
            }
            Action::D => {
                depth += movement;
            }
            Action::U => {
                depth -= movement;
            }
        }
    }

    return horizontal * depth;
}

fn main() {
    let inputs = read_inputs().expect("Input read correctly");
    let answer = part1(&inputs);
    println!("Answer: {}", answer)
}

Full solution with parse-display crate

use std::{
    fs::File,
    io::{BufRead, BufReader, Result as io_result},
    // str::FromStr,
};

use parse_display::{Display, FromStr};

#[derive(Display, FromStr)]
#[display("{action} {movement}")]
struct Step {
    action: Action,
    movement: u32,
}

#[derive(Display, FromStr)]
enum Action {
    #[display("forward")]
    F,
    #[display("down")]
    D,
    #[display("up")]
    U,
}

fn read_inputs() -> io_result<Vec<Step>> {
    let input_file = File::open("inputs/2.txt")?;
    let file_reader = BufReader::new(input_file);
    let inputs = file_reader
        .lines()
        .filter_map(|line| {
            let line_res = line.ok()?;
            // empty line
            if line_res.is_empty() { return None; }
            let res: Step = line_res.parse().expect("incorrect input line");
            Some(res)
        })
        .collect::<Vec<Step>>();
    return Ok(inputs);
}

fn part1(inputs: &Vec<Step>) -> u32 {
    let (mut horizontal, mut depth): (u32, u32) = (0, 0);

    for step in inputs {
        match step.action {
            Action::F => {
                horizontal += step.movement;
            }
            Action::D => {
                depth += step.movement;
            }
            Action::U => {
                depth -= step.movement;
            }
        }
    }

    return horizontal * depth;
}

fn main() {
    let inputs = read_inputs().expect("Input read correctly");
    let answer = part1(&inputs);
    println!("Answer: {}", answer)
}

Conclusion

  • Idiomatic, terse and fast Rust is often functional. Get to know functional style.
  • know your crates
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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