Advent of Code 2023, Day 8 Part 1 - Beginner Rust Solution

Question

Context

I'm doing Advent of Code as a way of learning Rust, and have a working solution to Day 8, Part 1. This involves moving between nodes, starting at AAA and looking for ZZZ, where the next node is determined by the L/R instruction chain in the first line. We loop over the L/R instruction set until ZZZ is found. Some of the example input is given below.

RL

AAA = (BBB, CCC)
BBB = (DDD, EEE)
CCC = (ZZZ, GGG)
DDD = (DDD, DDD)

I think the logic I used here is ok, but I expect my code to the solution does not take advantage of Rust's strengths. I know, for example, that I am being lazy by using .unwrap() instead of handling an Option.

My Code

use regex::Regex;
use std::collections::HashMap;
use std::fs;

const FILE: &str = "../inputs/day8_1.txt";

fn parse_inputs(filename: &str) -> (Vec<char>, HashMap<String, Vec<String>>) {
    let mut node_map: HashMap<String, Vec<String>> = HashMap::new();

    let file = fs::read_to_string(filename).expect("File read failure");

    let (directions, nodes) = file.split_once("\n\n").unwrap();

    let rgx = Regex::new(r"(\w{3})").unwrap();

    for line in nodes.lines() {
        let matches: Vec<String> = rgx
            .find_iter(line)
            .map(|x| x.as_str().to_string())
            .collect();

        node_map.insert(
            matches[0].clone(),
            vec![matches[1].clone(), matches[2].clone()],
        );
    }

    (directions.chars().collect::<Vec<char>>(), node_map)
}

fn main () {
    let mut step_count: u32 = 0;
    let mut point = "AAA".to_string();
    let (directions, data) = parse_inputs(FILE);
    let direction_map = HashMap::from([('L', 0), ('R', 1)]);
    let mut indexer: usize = 0;
    let max_index = directions.len() - 1;

    while point != "ZZZ".to_string() {
        let d: usize = direction_map[&directions[indexer]];
        point = data[&point][d].clone();
        if indexer < max_index {
            indexer += 1;
        } else {
            indexer = 0;
        }
        step_count += 1
    }

    println!("Part One: {:?}", step_count)
}

Review Request

I'm looking for advice on how this code might follow a more idiomatic Rust style. I think I am probably brute forcing my way through the compiler here, as I'm having to clone values to get past the borrowing rules. I suspect that I could write the code in a more intelligent way to avoid cloning. I also know that I could (and should) actually handle errors and options, but I'm not sure how best to do that here.

Answer 1

I honed my Rust through Advent of Code as well, so welcome to the club. I can tell you with hindsight that trying to handle errors in a formal way for early days with relatively simple input structures and problems is not really worth the hassle. The errors that could arise here are almost exclusively from the input itself, which is an essentially completely theoretical possibility in the case of AoC. At this point, getting used to using the ? operator in general is more important. Later on, once you'll be doing more complex operations where errors can actually practically arise from your own code, you can worry about being more methodical¹. In this case, you could simply make both parse_inputs and main return a Result<T, Box<dyn Error>> (where T is what they were returning before, or () in the case of nothing) and replace unwraps with ?.

With that out of the way, some more specific suggestions in no particular order:

• (directions.chars().collect::<Vec<char>>(), node_map) does not require an explicit type annotation (Vec<char>) because this is the return value and the type of the return value is explicitly specified already in the function's header. This also extends to node_map further up in the function for the same reason, and the definitions of step_count and indexer in the main function. Even more subtly, even when a type annotation is required, it may not need to be entirely explicit. For instance, when defining matches, you do need to specify that it is a Vec for collect. But the element types are already clear, so you can simply ask the compiler to infer it using an underscore: let matches: Vec<_>.

In general, explicit type annotations are actually fairly rare in Rust, and you should generally try removing them, even after a program compiles, just to see if they're necessary. Over time, you will intuit when they are likely to be needed and the friction will decrease.

• The Iterator trait is extraordinarily useful, and it is worth looking through it any time you're trying to do anything to do with iterators (which is essentially all of the time). In this case, Iterator::cycle can help make your main loop more clear. Instead of keeping track of an indexer and max_index, you can simply:

for dir in directions.iter().cycle() {
    let d: usize = direction_map[dir];
    point = data[&point][d].clone();

    step_count += 1
    if point == "ZZZ".to_string() {
        break;
    }
}

Opinions may vary as to the extent of this improvement, but in my view it makes what you're doing with directions more clear as it eliminates the mental overhead of 2 variables.

• Another general tip with Iterator is: don't collect too hastily. In the case of matches, you can avoid the overhead of a Vec by simply leaving the collect out and using .next().unwrap(). This is not just a matter of an altogether trivial optimization, as will hopefully be made clear.

let matches = rgx
    .find_iter(line)
    .map(|x| x.as_str().to_string());

let node = matches.next().unwrap();
let left = matches.next().unwrap();
let right = matches.next().unwrap();

node_map.insert(node, vec![left, right]);

Now you may argue that this introduces unwraps where there previously were none, which is true. However, since Vec indexing panics on index out of bounds, this in fact only makes the risk more explicit. These can now be converted to ?. (though not directly: next returns an Option, not a Result, but it is simple to convert between the two: e.g ok_or and related methods) This also eliminates 3 clones, which while not terribly expensive, are a bit unsightly.₂

• Related to the above point is that since left and right always form 2 elements, you can simply contain them in a tuple instead of a Vec: (String, String) at no cost to simplicity at all.

• The direction_map seems like a layer of unnecessary indirection to me where a simple if statement would be equivalent, but that is your choice to make, ultimately, it has little relevance to Rust idiomacity specifically.

• The other clone in your code, point = data[&point][d].clone() can be eliminated simply by removing the to_string in the definition of point, which downgrades it from a String to an &str. You can still compare &strs to each other, so it works fine.

---

Footnotes

1. When you do eventually need or want to get more methodical, see the crates anyhow and thiserror for approaches to error handling. But don't get sucked in too early: the practical differences between them will seem esoteric and not clearly motivated.

2. If you're not clear on how those clones were necessary and why they're gone now, Vec<T> indexing returns values of type &T, but using next on an Iterator<Item = T> explicitly consumes part of the iterator and so returns a T.