Code Review for Pig Latin

code review
1

Welcome to leave your comments. THANKS! :slight_smile:

  • whether there’s a better way
  • whether there’s a faster way
  • memory or ownership management
  • code safety
  • whether it’s idiomatic or not
  • code style/formatting
  • etc.
// Convert strings to pig latin. The first consonant of each word is moved to
// the end of the word and “ay” is added, so “first” becomes “irst-fay.” Words
// that start with a vowel have “hay” added to the end instead (“apple” becomes
// “apple-hay”). Keep in mind the details about UTF-8 encoding!
pub fn from(w: &str) -> Option<String> {
    if w.is_empty() {
        return None;
    }
    let bytes = w.as_bytes();
    if !bytes[0].is_ascii_alphabetic() {
        return None;
    }
    let vowels = vec![97_u8, 101_u8, 105_u8, 111_u8, 117_u8];
    if vowels.contains(&bytes[0].to_ascii_lowercase()) {
        return Some(format!("{}-hay", w));
    }
    let first_letter = String::from_utf8(vec![bytes[0]]).unwrap();
    let remained_letters = String::from_utf8(Vec::from(&bytes[1..])).unwrap();
    Some(format!("{}-{}ay", remained_letters, first_letter))
}
2

This one jumps out at me.

  • you can use b'a' to get ascii characters as bytes
  • if you never modify it, might as well just use an array literal like let vowels = [b'a', b'e', b'i', b'o', b'u'];
  • but then all the characters separately is a waste, and you can just do let vowels = b"aeiou";
3 Likes
4

My suggestion:

pub fn from(w: &str) -> Option<String> {
    if w.is_empty() {
        // Don't "return None;""
        // I think this makes more sense
        // remember that 0-sized strings
        // do not allocate so this is not
        // really expensive
        return Some(String::new());
    }
    // Here I have remove some redundant indexing.
    let first_byte = w.as_bytes()[0];
    if !first_byte.is_ascii_alphabetic() {
        return None;
    }
    // as said before, vowel = b"abeiou" is better
    //let vowels = vec![97_u8, 101_u8, 105_u8, 111_u8, 117_u8];
    let vowels = b"abeiou";
    if vowels.contains(&first_byte.to_ascii_lowercase()) {
        return Some(format!("{}-hay", w));
    }
    // Oooh, way too much allocations here, one for each vec and String, then
    //for the format
    //let first_letter = String::from_utf8(vec![bytes[0]]).unwrap();
    //let remained_letters = String::from_utf8(Vec::from(&bytes[1..])).unwrap();
    //Some(format!("{}-{}ay", remained_letters, first_letter))
    // Rather do:
    let (first_letter,suffix) =  w.split_at(1);
    Some(format!("{}-{}ay",suffix,first_letter))
}

Playground: Rust Playground

1 Like
5

Explain why to return None for empty string:

This exercise is from Chapter 8 of The Book, the question does not specify what to return if it can not be a Pig Latin, so I think there are two choices I can take:

  • Return the string as it is, and the result type only needs to be String, but it can't express the meaning of whether the result is a PigLatin.
  • Any string that can't be PigLatin will return None variant, so I think an empty string should apply this rule.
6

You are entirely right, both solutions make sense.

7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.