1

This code works, but is extremely ugly. This is a question from exercism.org under the Rust track. How can I avoid using try_into().unwrap() everywhere?

What am I doing wrong, and how can I learn to write more idiomatic code?

pub struct Allergies {
    score: u32,
}

#[derive(Debug, PartialEq, Copy, Clone)]
pub enum Allergen {
    Eggs = 1,
    Peanuts = 2,
    Shellfish = 4,
    Strawberries = 8,
    Tomatoes = 16,
    Chocolate = 32,
    Pollen = 64,
    Cats = 128,
}

impl Allergies {
    pub fn new(score: u32) -> Self {
        Allergies { score }       
    }

    pub fn is_allergic_to(&self, allergen: &Allergen) -> bool {
        let mut score = self.score;
        score %= 256;
        for i in (0..8).rev() {
            if 2_i32.pow(i.try_into().unwrap()) != *allergen as i32 && score >= 2_u32.pow(i.try_into().unwrap()) {
                score -= 2_u32.pow(i.try_into().unwrap());
            } else if 2_i32.pow(i.try_into().unwrap()) == *allergen as i32 && score >= 2_u32.pow(i.try_into().unwrap()) {
                return true;
            }
        }
        false
    }

    pub fn allergies(&self) -> Vec<Allergen> {
        let allergens:[Allergen; 8] = [Allergen::Eggs, Allergen::Peanuts, Allergen::Shellfish, Allergen::Strawberries, Allergen::Tomatoes, Allergen::Chocolate, Allergen::Pollen, Allergen::Cats];
        let mut allergies = Vec::<Allergen>::new();
        for allergen in allergens.iter() {
            if self.is_allergic_to(allergen) {
                allergies.push(*allergen)
            }
        }
        allergies
    }
}

Notice this bit which is terrible:

            if 2_i32.pow(i.try_into().unwrap()) != *allergen as i32 && score >= 2_u32.pow(i.try_into().unwrap()) {
                score -= 2_u32.pow(i.try_into().unwrap());
            } else if 2_i32.pow(i.try_into().unwrap()) == *allergen as i32 && score >= 2_u32.pow(i.try_into().unwrap()) {
                return true;
            }

How can I rewrite this whole thing to be more beautiful?

Nate Houk
  • 335
  • 1
  • 10
  • 2
    Did you try simply removing `.try_into().unwrap()`? [It seems to work for me.](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2a39ca1b002b338b47e40d492dc9b231) – user4815162342 Feb 21 '22 at 13:04
  • OK, that works. Not sure how I ended up with the code above, was fighting with the compiler with the u32 and i32 casting. – Nate Houk Feb 21 '22 at 13:12
  • 1
    It was probably needed in a previous version of the code. Also, you can replace `.try_into().unwrap()` with a much simpler `as u32`. The compiler suggests the more verbose variant in an attempt to make handling of negative numbers more transparent (i.e. panicking instead of silently overwrapping). – user4815162342 Feb 21 '22 at 13:14
  • Also, you can probably replace `2.pow(i)` with `1 << i`. – user4815162342 Feb 21 '22 at 13:15
  • 1
    You can also get rid of all the `_u32` and `_i32` suffixes if you use `1< – Jmb Feb 21 '22 at 13:16

1 Answers1

2

Found some way more idiomatic code... key is doing a bitwise and against the enum and the score, and then using a .filter() when collecting the items in the end.

    pub fn is_allergic_to(&self, allergen: &Allergen) -> bool {
        let allergen_score = *allergen as u32;
        (self.score() & allergen_score ) == allergen_score
    }

    pub fn allergies(&self) -> Vec<Allergen> {
        ALLERGENS.iter()
                 .filter( |a| self.is_allergic_to(a) )
                 .cloned()
                 .collect()
    }
Nate Houk
  • 335
  • 1
  • 10
  • In other posts I was reading that using `as` is not safe. Refer to the comments in the accepted answer here https://stackoverflow.com/questions/43704758/how-to-idiomatically-convert-between-u32-and-usize – adam2k Jul 11 '23 at 20:51