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:
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:
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(' ');
}
Another of these, you noticed yourself and asked for feedback on:
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:
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 reality, rustc is able to optimize away the extra copy when you write:
result = result + word + "-hay";
So this is, in reality, 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 has significant overhead.
The interface you actually want to do this efficiently is not taught in the Rust Book: str::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.