5
\$\begingroup\$

I'm doing a simple task in Rust after reading the Rust book:

  • read a text file
  • split it at whitespace
  • sanitize words by removing non-letter characters
  • sort the words by their frequency of occurrence

Here's what I've got, which works, but I'd like some advice:

  • are there any unnecessary implicit copies that could be replaced by moves?
  • are there nice ways to avoid let mut?

 

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

lazy_static! {
    static ref NON_LETTER_RE: Regex = Regex::new("[^a-zA-Z]").unwrap();
}

fn sanitize(s: &str) -> String {
    NON_LETTER_RE.replace_all(s, "").into_owned().to_lowercase()
}

fn main() {
    let words = std::fs::read_to_string("data.txt").unwrap();
    let words = words.split_whitespace().collect::<Vec<_>>();

    let mut freqs = HashMap::new();
    for word in words {
        *freqs.entry(sanitize(word)).or_insert(0) += 1;
    }

    let mut freqs = freqs.into_iter().collect::<Vec<_>>();
    freqs.sort_by(|(_, a), (_, b)| b.cmp(a));

    for (a, b) in freqs {
        println!("{}: {}", a, b);
    }
}
\$\endgroup\$
3
\$\begingroup\$

This is a very solid attempt!

Really, the only negative comment I have is that you collect words into a Vec and then immediately iterate over it. Instead, you can just use the iterator directly.

Regarding avoiding let mut, there's nothing specific to your code I can think of, but there are a few general methods to avoid it.

The first is to make a block and return it from that:

let v = {
    let mut v = vec![2, 3, 1];
    v.sort();
    v
}

The second is to rebind as immutable.

let mut v = vec![2, 3, 1];
v.sort();
let v = v;

The third is to use a crate like tap, which provides a trait implemented by everything which provides a tap method.

let v = vec![2, 3, 1].tap(|mut v| v.sort());

Next, I have some suggestions for alternatives and personal preferences.

Rather than splitting on whitespace then sanitizing words, you could use regex's match function and select all continuous sequences of letters. This has slightly different semantics, since it would separate e.g. "can't" into two words. However, you could instead also allow ' and - in a word. By instead scanning by regex with find_all, you can specify exactly what constitutes a word.

Instead of or_insert(0), you can use or_default(). Since the default for numbers is 0, This does the same exact thing, but is a bit cleaner.

When collecting, I prefer to annotate the type of the variable (let x: T) rather than using turbofish notation (::<T>), I believe it looks cleaner.

The way you're sorting is perfectly fine, but I prefer using sort_by_key when possible (and reasonable). In this case, you can just wrap the key in std::cmp::Reverse, a helpful struct that reverses the ord instance on it's contents.

In general, I don't see any unneeded clones/allocations. Since the hashmap is so temporary, I would typically suggest making it's key a &str, but you can't do this since you have to make your keys lowercase. However, there's a clever way to get around this. You can wrap your &strs in a helper struct that does case insensitive hash and eq. Luckily, there's a crate for this called unicase, with structs for unicode-case-insensitive and ascii-case-insensitive String/&str. By doing this, you eliminate the allocation of a String for each word. Note that this will make your words have the case of the first time they occur, so you may want to lowercase them before printing them.

All together, here's how I might write this program.

use std::cmp::Reverse;
use std::collections::HashMap;

use unicase::Ascii;
use lazy_static::lazy_static;
use regex::Regex;

lazy_static! {
    static ref WORD: Regex = Regex::new("[-'a-zA-Z]+").unwrap();
}

fn main() {
    let data = std::fs::read_to_string("data.txt").unwrap();
    let words = WORD.find_iter(&data);

    let mut freqs: HashMap<Ascii<&str>, usize> = HashMap::new();
    for word in words {
        *freqs.entry(Ascii::new(word.as_str())).or_default() += 1;
    }

    let mut freqs: Vec<_> = freqs.into_iter().collect();
    freqs.sort_by_key(|&(_, freq)| Reverse(freq));

    for (word, freq) in freqs {
        println!("{}: {}", word, freq);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the detailed suggestions! I had tried using sort_by_key(|(_, freq)| freq) at first, but the compile error was a bit inscrutable. Can you explain why this doesn't work with |(_, freq)| but does with |&(_, freq)|, while my original code |(_, a), (_, b)| does work? \$\endgroup\$ – jtbandes Mar 2 at 3:06
  • 1
    \$\begingroup\$ Here's a good answer for that. \$\endgroup\$ – JayDepp Mar 2 at 6:11

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.