Code Review for Pig Latin

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))

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";

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);

Playground: Rust Playground

1 Like

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.

You are entirely right, both solutions make sense.

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.