Not a bad start. Some ways it could be improved: ### Use `&str` for String Slices Currently, you cannot call `to_pig_latin("apple")`, because the input is a `&String`. It requires `to_pig_latin(&String::from("apple"))`. You don’t need to modify or move the input, so the signature should be pub fn to_pig_latin(phrase: &str) -> String Now it will understand all stringy types, including a `String`, a `&str` or a string literal. ### There are Some Bugs Try this with an empty input string, one with all spaces, or one starting with capital A, E, I, O or U. In fact: ### Write Test Cases Maybe you did, but there are none included here. A basic start: ```lang-rust pub fn main() { assert_eq!(to_pig_latin( "a b c d e f g h i j k l m n o p q r s t u v w x y z"), "a-hay -bay -cay -day e-hay -fay -gay -hay i-hay -jay -kay -lay -may -nay o-hay -pay -qay -ray -say -tay u-hay -vay -way -xay -yay -zay" ); assert_eq!(to_pig_latin( "Australian babies complain dingoes eat flavorful girls hungrily in jumping kangaroo lairs munching no other poorer quoditian refreshment so teach utterly verily when xpounding youths zoology"), "Australian-hay abies-bay omplain-cay ingoes-day eat-hay lavorful-fay irls-gay ungrily-hay in-hay umping-jay angaroo-kay airs-lay unching-may o-nay other-hay oorer-pay uoditian-qay efreshment-ray o-say each-tay utterly-hay erily-vay hen-way pounding-xay ouths-yay oology-zay"); assert_eq!(to_pig_latin(""), ""); assert_eq!(to_pig_latin(" "), ""); } ``` ### Don’t Make Expensive Copies You Don’t Need The biggest problem with the efficiency of this program is that it creates a lot of temporary strings, does round-trip conversions from the native UTF-8 format to 32-bit `char` and back, copies all but the last character of the `result` string into a new `String` object, and so on. Each of these needs to create an object, perform an allocation, and copy some data in linear time. A good example is the implementation of `is_vowel`: ```lang-rust fn is_vowel(c :char)-> bool{ c.to_string() == String::from("a") || c.to_string() == String::from("e") || c.to_string() == String::from("i") || c.to_string() == String::from("o") || c.to_string() == String::from("u") } ``` This creates two `String` objects at runtime and compares them. You could just have compared (c == 'a') || (c == 'e') || (c == 'i') || (c == 'o') || (c == 'u') || (c == 'A') || (c == 'E') || (c == 'I') || (c == 'O') || (c == 'U') Or written the `if is_vowel(first_char)` test as a `match` expression: match first_char { 'a' | 'e' | 'i' | 'o' | 'u' | 'A' | 'E' | 'I' | 'O' | 'U' => { unimplemented!(); // You want to write these yourself. } 'b' | 'c' | 'd' | 'f' | 'g' | 'h' | 'j' | 'k' | 'l' | 'm' | 'n' | 'p' | 'q' | 'r' | 's' | 't' | 'v' | 'w' | 'x' | 'y' | 'z' => { unimplemented!(); // You want to write these yourself. } 'B' | 'C' | 'D' | 'F' | 'G' | 'H' | 'J' | 'K' | 'L' | 'M' | 'N' | 'P' | 'Q' | 'R' | 'S' | 'T' | 'V' | 'W' | 'X' | 'Y' | 'Z' => { unimplemented!(); // You want to write these yourself. } _ => { unimplemented!(); // You want to write these yourself. } } If you really, truly do need a comparison like this, at least compare to a `const` or `static` value or a literal, instead of creating a new temporary. Similarly, instead of copying all but one `char` of `result` to a new `String`, you would be better off truncating `result`, or using a different approach, such as: if !result.is_empty() { result.push(' '); } or: let separator : &str = if result.is_empty() { "" } else { " " }; Another of these, you noticed yourself and asked for feedback on: ```lang-rust result += &(pig_latin_word.to_owned() + "-hay "); ``` The reason this needed `.to_owned()` is that you can’t add string slices—only a slice **to** a `String` (so `.to_string()` or `String::from` would also have worked). There’s no space at the end of a slice to add anything to, and no way to resize it. So you end up creating a temporary `String` on the right hand side of `+=`, then converting that to a slice. Not only does this look ugly, it ends up copying all the bytes that you append, twice. One way around this would be to write: ```lang-rust result += word; result += "-hay"; ``` (You don’t need `pig_latin_word`, since you just set it to `word` and never modify it, but this copy is harmless.) However, there’s a nicer alternative. Even though this more fluent style *looks* as if it would need to make a temporary copy of `result`, in fact, rustc is able to optimize away the extra copy when you write: ```lang-rust result = result + word + "-hay"; ``` So this is a zero-cost abstraction. The reason this works is that, in Rust (unlike C++ or Java), adding something to a string consumes the string on the left side of the `+`. (Therefore, if you do want to add something to a `String` and also use the original again, you want something like `let hello_world = hello.clone() + ", world!";`.) ### Work on Slices Instead Whenever possible, you want to be working with slices of the underlying array of bytes, which can be created, resized or passed around in constant time. This is a bit tricky to do here, because a Rust `String` is, internally, **not** an array of `char` like in many other languages. It’s really an array of `u8` holding bytes encoded in UTF-8, so you do not have random access to each `char`, only sequential. Therefore, concatenating string slices is a zero-cost abstraction, but iterating over each `char` in a string is not: it has to convert between UTF-8 and UCS-4. The interface you actually want to do this efficiently is not taught in the Rust Book: [`str::char_indices`](https://doc.rust-lang.org/std/primitive.str.html#method.char_indices). So here’s a hint: let mut indices = word.char_indices(); if let Some((0, first_char)) = indices.next() { let mid = match indices.next() { None => word.len(), Some((i, _)) => i, }; let (first_slice, rest_of_word) = word.split_at(mid); /* Now first_slice is a slice containing the first letter of the word, * and rest_of_word is a (possibly-empty) slice containing the rest of the * word. As before, first_char is the first Unicode character of the * word as a 32-bit char. You decide what to do with them here. */ } else { unreachable!("A word was not valid UTF-8"); } ### Consider Refactoring with a Helper Function Currently, you have a `for` loop that iterates over the words in the input as string slices. This is a good approach! But there are alternatives that you’ve seen if you’ve done functional programming before, but not in the Rust Book so far. You can start by factoring out the body of the loop into a helper function, which can be nested inside `to_pig_latin`: fn helper(mut accumulator: String, word: &str) -> String What this should do is append the Pig-Latin translation of the `word` slice to the `accumulator` string, separated by a space, and return the updated accumulator. (You could also implement this without a `mut` accumulator.) Doing it this way avoids creating or copying any `String` other than `accumulator`. This can simplify your `for` loop, but the real benefit of this is that there’s a very elegant abstraction for iterating over a sequence and passing each item to a function like this, then returning the final value. The body of the `to_pig_latin` function can become simply phrase.split_whitespace() .fold(String::new(), helper) Plus of course the definition of `fn helper`. I personally strongly prefer this style over a `for` loop.