Hacker News new | ask | show | jobs
by DougBTX 2622 days ago
The output form this is a little different form the article, as it additionally requires:

* that the uniqueness check is done case-insensitively,

* that the casing from the first instance of the string in the input array should be used in the output, and

* that the output should be ordered based on the order of the first instance of the string in the input array

Possibly an unusual set of requirements (I'd generally expect that if removing duplicates from a list, that the output order wouldn't matter) but hey, different problems have different requirements.

The desired output is:

    apple
    cAt
    Dog
    banana
while that code snippet produces:

    apple
    cAt
    cat
    Dog
    dog
    Cat
    Apple
    dOg
    banana
2 comments

I've tried to fix the Rust code, but I'm getting some lifetime errors. Can anyone spot the problem? (Note: I've used C++ for most of the time and am still uncomfortable with Rust)

  use std::collections::HashSet;

  fn main () {
    let strings = vec![
      "apple", "cAt", "cat", "Dog", "apple", "dog", "Cat",
      "Apple", "dOg", "banana", "cat", "dog", "apple",
    ];

    let mut set : HashSet<&'static str> = HashSet::new();
    
    let strings : Vec<&str> = strings
        .into_iter()
        .map(|string| (string, string.to_lowercase()))
        .filter(|(_, lower)| set.insert(lower))
        .map(|(string, _)| string)
        .collect();
    
    println!("{:?}", strings);
  }
The lower value is of type &String because the argument to filter is of type &(String, String) and the pattern match needs to propagate the &. The reference is only valid for the body of the filter closure (that is, it is &'short String, for some anonymous/unnamed lifetime 'short), but it is being put into a set that expects &'static str (the explicit type on the let line). The &String to &str type coercion is fine, but it's not correct to treat the &'short as a &'static (the short one is potentially invalid after filter's closure returns).

Changing the type annotations won't make this compile, because it is actually catching a real bug. The iterator is lazy and the full pipeline is executed for each element at once: lowercasing, inserting into the set, inserting into the Vec that's the result of the collect, and deallocating the lower string. This last step is the key/danger: if the HashSet held references/slices to the lower strings (instead of owning them), those references would become dangling immediately and future look-ups into the set won't work right/will trigger undefined behaviour.

The problem is a little clearer (and mostly fixed) if you simplify the code slightly by removing the two map calls, and instead call to_lowercase in the filter directly:

        .into_iter()
        .filter(|string| set.insert(string.to_lowercase()))
        .collect();
This form is a type error, that can be corrected by changing the type annotation to be HashSet<String>, or even removing it entirely and letting type inference handle it. The HashSet owning the strings is the key, so they only disappear after the entire iteration is complete, not after each element.
Even with the hash set solution, it would be nice to not copy the string contents, which unfortunately is needed if the hash set contains lowercased versions of the strings. On the other hand, converting all strings to lowercase then comparing them isn't a great way to case-insensitively compare strings anyway, as Unicode doesn't guarantee that will work.

The UniCase crate defines a wrapper around strings with a case-insensitive Eq implementation, so this works:

    use std::collections::HashSet;
    use unicase::UniCase;

    fn main () {
        let strings = vec![
            "apple", "cAt", "cat", "Dog", "apple", "dog", "Cat",
            "Apple", "dOg", "banana", "cat", "dog", "apple",
        ];
        
        let mut set = HashSet::new();
        
        let strings: Vec<_> = strings
            .iter()
            .filter(|&string| set.insert(UniCase::new(string)))
            .collect();
        
        println!("{:?}", strings);
    }
https://play.rust-lang.org/?version=stable&mode=debug&editio...
This is pretty cool. People might think it's cheating to pull in a crate. But IMO the fact that its super easy to pull in a crate in Rust is one of the best things about the language.
You don't even need the filter, all you need is:

    let dedup = strings.into_iter().map(|s| s.to_lowercase()).collect::<HashSet<_>>().into_iter().collect::<Vec<_>>();
Unfortunately that doesn't preserve case nor order of the input strings.
Similar to the C++, all of those are addressed by inserting lower-cased into the set, which can be achieved via an extra method call:

   set.insert(string.to_lowercase())
It also requires adjusting/deleting the type annotation on the set.

https://play.rust-lang.org/?version=stable&mode=debug&editio...