2
\$\begingroup\$

I'm learning Rust and one of the challenges in the book was to build a pig latin translator.

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

I came up with the code below and I'm looking for ways to improve it, specifically reducing the push_strs and improving the starts_with_a_vowel method. How can the code be improved?

main.rs

extern crate unicode_segmentation;
use unicode_segmentation::UnicodeSegmentation;

fn main() {
    println!("{}", pig_latinify("String me".to_string()));
}

fn transform_word(word: &str) -> String {
    let mut new_word = String::new();
    new_word.push_str(&word[1..]);
    new_word.push_str(&word[..1]);
    if starts_with_a_vowel(word) {
        new_word.push_str("h");
    }
    new_word.push_str("ay");
    new_word
}

fn starts_with_a_vowel(word: &str) -> bool {
    let vowels = ["a", "e", "i", "o", "u"];
    for vowel in &vowels {
        if word.starts_with(vowel) {
            return true;
        }
    }
    false
}

fn pig_latinify(statement: String) -> String {

    let mut transformed_statement = String::new();
    if !statement.is_ascii() {
        panic!("statement must be ascii");
    }
    let iter = statement.split_word_bounds();
    for word in iter {
        if word.trim().len() > 0 {
            transformed_statement.push_str(&transform_word(word));
            transformed_statement.push_str(" ");
        }
    }
    transformed_statement.trim();
    transformed_statement
}

Cargo.toml

...
[dependencies]
unicode-segmentation = "1.2.0"
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Write tests

Unfortunately, your piglatify doesn't follow your rules. apple gets transformed into applyaay, but it should get transformed into apple-hay. Also, first should get transformed into irst-fay, but you will transform it into irstfay.

Rust makes it really easy to add tests to your code:

#[cfg(test)]
mod test {
    use transform_word;

    #[test]
    fn empty_string() {
        assert_eq!(transform_word(""), "");
    }

    #[test]
    fn apple() {
        assert_eq!(transform_word("apple"), "apple-hay");
    }

    #[test]
    fn first() {
        assert_eq!(transform_word("first"), "irst-fay");
    }
}

While this only uses the given strings from the exercise, it already tells us that there is something amiss if we run the tests with cargo test:

running 3 tests
test test::empty_string ... FAILED
test test::first ... FAILED
test test::apple ... FAILED

failures:

---- test::empty_string stdout ----
    thread 'test::empty_string' panicked at 'byte index 1 is out of bounds of ``', /checkout/src/libcore/str/mod.rs:2217:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- test::first stdout ----
    thread 'test::first' panicked at 'assertion failed: `(left == right)`
  left: `"irstfay"`,
 right: `"irst-fay"`', src/main.rs:57:9

---- test::apple stdout ----
    thread 'test::apple' panicked at 'assertion failed: `(left == right)`
  left: `"ppleahay"`,
 right: `"apple-hay"`', src/main.rs:52:9

Use with_capacity if possible

You can use String::with_capacity in transform_word. The new word has the same length as the old one plus four:

fn transform_word(word: &str) -> String {
    if word.is_empty() {
        return String::new();
    }
    let mut new_word = String::with_capacity(word.len() + 4);

    // still buggy, see above
    new_word.push_str(&word[1..]);
    new_word.push_str(&word[..1]);
    if starts_with_a_vowel(word) {
        new_word.push_str("h");
    }
    new_word.push_str("ay");
    new_word
}

However, we still need to fix the bug mentioned before:

fn transform_word(word: &str) -> String {
    if word.is_empty() {
        return String::new();
    }
    let mut new_word = String::with_capacity(word.len() + 4);

    if starts_with_a_vowel(word) {
        new_word.push_str(&word);
        new_word.push_str("-hay");
        new_word
    } else {
        new_word.push_str(&word[1..]);
        new_word.push('-');
        new_word.push_str(&word[..1]);
        new_word.push_str("ay");
        new_word
    }
}

Alternative variants

Truth be told, that looks somewhat silly. We can just use += instead of push_str, or even +:

fn transform_word(word: &str) -> String {
    if word.is_empty() {
        return word.to_owned();
    }
    if starts_with_a_vowel(word) {
        word.to_owned() + "-hay"
    } else {
        word[1..].to_owned() + "-" + &word[..1] + "ay"
    }
}

Speaking about alternatives, if you use any then starts_with_a_vowel can be written in a single line:

fn starts_with_a_vowel(word: &str) -> bool {
    "aeiou".chars().any(|vowel| word.starts_with(vowel))
}

Note that we use a Char as pattern, not a String. starts_with works with both, but this variant shows our intend to match only a single character.

Ownership

It's not necessary to take ownership of the given String in pig_latinify.

\$\endgroup\$
2
  • \$\begingroup\$ Yup that is definitely what I was going for. I changed transformed_statement.push_str's to transformed_statement += &transform_word(word); transformed_statement += " "; Is it possible to combine these into one statement? \$\endgroup\$
    – topher
    Mar 29, 2018 at 12:20
  • \$\begingroup\$ @topher join(" ") or one of these methods. \$\endgroup\$
    – Zeta
    Mar 29, 2018 at 12:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.