5
\$\begingroup\$

I tried solving this /r/dailyprogrammer challenge in Rust.

I'm a complete Rust beginner, and I'm sure I've made a lot of mistakes. I would be very thankful for any tips or help you might be able to provide.

Here are a couple of points that I found awkward/confusing:

  • In show_result, what I basically wanted to do was something like

    symbols = spelling.map {|e| e.symbol}.join("")
    elements = spelling.map {|e| e.name}.join(", ")        
    

    but that didn't seem to work.

  • I'm not exactly sure why I have to clone the elements to put them into the result vector. Can I/should I avoid this?
  • Coming from a dynamically typed garbage collected language, I had a hard time deciding when and where to use references.

Thank you very much!

extern crate csv;

use std::env;
use std::path::Path;
use std::collections::HashMap;
use std::option::Option;
use std::vec::Vec;

#[derive(Clone)]
struct Element {
    number: u16,
    symbol: String,
    name: String
}

impl Element { 
    fn new (number: u16, symbol: &str, name: &str) -> Element {
        Element { 
            number: number,
            symbol: symbol.to_string(),
            name: name.to_string()
        }
    }
}

// Read all elements from CSV file into a hash map
fn read_elements() -> HashMap<String, Element> {
    let mut elem_map = HashMap::new();

    let path = Path::new("./elements.csv");
    let mut csv_reader = csv::Reader::from_file(path).unwrap();
    for record in csv_reader.decode() {
        let (number, symbol, name): (u16, String, String) = record.unwrap();
        let symbol = symbol.trim();
        let name = name.trim();
        elem_map.insert(symbol.to_lowercase().to_string(), Element::new(number, symbol, name));
    };

    return elem_map;
}

// Try to spell target_word using elements from elem_map
fn spell_word(target_word: &str, elem_map: HashMap<String, Element>) -> Option<Vec<Element>> {
    let mut result: Vec<Element> = Vec::new();
    let target_word = target_word.to_lowercase().to_string();

    // How far we've matched so far
    let mut current: usize = 0;

    while current < target_word.len() {
        // Try matching the next two letters first
        if current + 2 <= target_word.len() {
            let next_two = &target_word[current..current + 2];
            if let Some(ref elem) = elem_map.get(next_two) {
                result.push((*elem).clone());
                current += 2;
                continue;
            }
        }

        // Try matching one letter if two letters failed
        let next_one = &target_word[current..current + 1];

        if let Some(ref elem) = elem_map.get(next_one) {
            result.push((*elem).clone());
        } else {
            // If no one letter element could be found,
            // this word cannot be spelled using elements
            return None;
        }
        current += 1;
    }

    return Some(result);
}

// Print the found spelling
// e.g. PoISON (polonium, iodine, sulfur, oxygen, nitrogen)
fn show_result(spelling: Vec<Element>) {
    let mut symbols: String = String::new();
    let mut elements: Vec<String> = Vec::new();

    for elem in spelling {
        symbols.push_str(&elem.symbol);
        elements.push(elem.name.to_lowercase());
    }

    println!("{} ({})", symbols, elements.join(", "));
}

fn main() {
    let args: Vec<String> = env::args().collect();
    let elements = read_elements();

    if args.len() <= 1 {
        println!("Please provide a word to spell.");
    } 

    let target_word: &str = &args[1];

    match spell_word(target_word, elements) {
        Some(spelling) => show_result(spelling),
        None => println!("Could not find a match for word \"{}\".", target_word)
    }
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Overall

  1. Don't explicitly import Vec or Option, they are part of the prelude.
  2. Basically always want to implement Debug on any type.
  3. I see two types that aren't there: something that holds the elements, something that contains results.
  4. This allows you to implement Display for Spelling. This is more flexible and participates in the ecosystem better.

read_elements

  1. No need to use Path explicitly, &str implements AsRef<Path>.
  2. Can collect iterator of tuples into a HashMap, no need to muck about with a mutable variable.
  3. Don't use unwrap. Prefer expect or return a Result. expect has words that will help you or the user find the problem. Note that we can collect into a Result.
  4. Don't hardcode the filename inside the function.

spell_word

  1. Should not take elem_map by value. Take a reference as we aren't making use of the allocated data.
  2. I'd split the string and just track the string pieces, not the distance into the string.
  3. No explicit return at the end of function.
  4. Return references to the element instead of cloning.

show_result

  1. Don't specify the types on a variable binding; let inference take care of it.
  2. Don't take ownership of Vec, instead take a slice.
  3. I'd avoid allocating in the printing function. I used Itertools to help with the comma separating.

main

  1. Don't specify the type of a collection, let inference take care of it (let foo: Vec<_> = bar.collect()).
  2. Don't check for the argument and then implicitly check again (via indexing). A match or unwrap would be better.
  3. No need to collect into a vector. Instead, skip the program name and just iterate over all the arguments - bonus feature!
  4. No need to explicitly convert String to &str, a &String can be coerced to a &str.
  5. Use raw strings to avoid escaping quotes
extern crate csv;
extern crate itertools;

use std::{env, fmt};
use std::collections::HashMap;

use itertools::Itertools;

type Error = Box<std::error::Error>;

#[derive(Debug, Clone)]
struct Element {
    number: u16,
    symbol: String,
    name: String
}

impl Element {
    fn new (number: u16, symbol: &str, name: &str) -> Element {
        Element {
            number: number,
            symbol: symbol.to_string(),
            name: name.to_string()
        }
    }
}

struct Elements {
    elem_map: HashMap<String, Element>
}

impl Elements {
    fn read_from_file(name: &str) -> Result<Self, Error> {
        let mut csv_reader = csv::Reader::from_file(name)?;

        let elem_map: Result<_, Error> = csv_reader.decode().map(|record| {
            let (number, symbol, name): (u16, String, String) = record?;
            let symbol = symbol.trim();
            let name = name.trim();
            Ok((symbol.to_lowercase().to_string(), Element::new(number, symbol, name)))
        }).collect();

        Ok(Elements { elem_map: elem_map? })
    }

    // Try to spell target_word using elements from elem_map
    fn spell_word(&self, target_word: &str) -> Option<Spelling> {
        let mut result = Vec::new();
        let target_word = target_word.to_lowercase().to_string();
        let mut target_word = target_word.as_str();

        while !target_word.is_empty() {
            // Try matching the next two letters first
            if target_word.len() >= 2 {
                let (head, tail) = target_word.split_at(2);

                if let Some(elem) = self.elem_map.get(head) {
                    result.push(elem);
                    target_word = tail;
                    continue;
                }
            }

            // Try matching one letter if two letters failed
            let (head, tail) = target_word.split_at(1);

            if let Some(elem) = self.elem_map.get(head) {
                result.push(elem);
                target_word = tail;
                continue
            }

            // If no one letter element could be found,
            // this word cannot be spelled using elements
            return None;
        }

        Some(Spelling(result))
    }
}

struct Spelling<'a>(Vec<&'a Element>);

impl<'a> fmt::Display for Spelling<'a> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        for elem in &self.0 {
            write!(f, "{}", elem.symbol)?;
        }

        write!(f, " (")?;

        for elem in self.0.iter().map(|e| e.name.as_str()).intersperse(", ") {
            write!(f, "{}", elem)?;
        }

        write!(f, ")")
    }
}

fn main() {
    let elements = Elements::read_from_file("./elements.csv").expect("Unable to read elements");

    for target_word in env::args().skip(1) {
        match elements.spell_word(&target_word) {
            Some(spelling) => println!("{}", spelling),
            None => println!(r#"Could not find a match for word "{}"."#, target_word)
        }
    }
}
\$\endgroup\$
0

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.