Being myself new to rust, take the following with a grain of salt. I would welcome further edits.
I think, generally, that there are two things your code is trying to do.
On one hand, it's about string tokens and concatenation.
On the other, there's the solving the 99 bottles problem.
99 Bottles
For this problem, your approach seems complex and results in rather gauche
code. What makes it so, is that it's hard to understand at first glance what
phrase
is at any point.
Adding comments would be a good thing.
Myself, I would forego the whole tokenisation part, and solve it in a more
concise way.
There are only a handful of sentences in the song, which need slight variations.
I'd like to use something like Python's raw strings:
SOLO = r"{QTY} bottles of beer on the wall, {QTY} bottles of beer."
SOLOT.format(QTY=count)
In rust, I managed to get this behaviour by using std::String::replace
:
const SOLO: &str = "{QTY} bottles of beer on the wall, {QTY} bottles of beer."
let mut solo: String = SOLO.to_string().replace("{QTY}", &count.to_string());
Once can likewise replace bottles
for its singular form, one
for it
.
Other details to clarify are:
1. are negative bottles allowed?
2. in one place, we're talking of phrase, in another of verse, and returned are
a bunch of sentences. Which nomenclature is correct? A single term ought to be used.
3. We're only dealing with beer, perhaps bottles_of_beer
could become bottles
?
We then end up with something like so:
const SOLO: &str = "{QTY} bottles on the wall. ";
const CHORUS: &str = "Take one down and pass it around, {QTY} bottles of beer on the wall.\n";
const BOTTLE: &str = "bottle"; // for substitution when 1 bottle left.
const NONE: &str = "\
No more bottles of beer on the wall, no more bottles of beer.\n\
Go to the store and buy some more, {QTY} bottles of beer on the wall.\n\
";
pub fn get_song(start: u32, stop: u32) -> String {
let mut song: String = String::new();
for x in (start..stop+1).rev() {
if x == 0 {
let no_beer: String = NONE.to_string().replace("{QTY}", &start.to_string());
song = song + &no_beer;
break;
}
let mut solo: String = SOLO.to_string().replace("{QTY}", &x.to_string());
let mut chorus: String = CHORUS.to_string().replace("{QTY}", &x.to_string());
if x == 1 {
solo = solo.replace("bottles", BOTTLE);
chorus = chorus.replace("bottles", BOTTLE);
chorus = chorus.replace("one", "it");
}
song = song + &solo + &chorus;
}
return song;
}
fn main() {
println!("{}", get_song(10, 0));
}
note that I am not sure this is a correct way to do such constant
strings.
I think let mut song: String = String::new();
is the preferred style,
as seen in the rust book, over
let mut phrase: String;
The while
loop can be replaced by a for
loop. This is actually the textbook example:
for x in (stop..start+1).rev() {
...
}
There's the +1
increment to include the starting value, then the iterator is reversed to go from high to low values.
String Tokens and Concatenation
To stay on the string concatenation path, I suggest commenting what each path does.
There are several if n < 1
, which could be put together. Within that, n
is redefined and subtracted, leading to an nested if n < 1
test.
We can observe that there are two types of operations: building different types of sentences,
and checking whether we're at the end of the song (n < 1
).
Let's agree to not return a phrase
, but a paragraph
:
let mut paragraph: String = String::new();
// Build qty of bottles on the wall sentence
if n < 1 {
...
paragraph = format!(...);
} else { ... }
// Build qty of bottles left on the wall sentence
if n < 1 {
paragraph = format!("{} {}", paragraph, ...);
} else { ... }
And so on.
Now I'm thinking, what's actually done here, is not checking a condition, but
rather match
ing a parameter, so if-else
s could become:
// Build qty of bottles on the wall sentence
match n {
0 => paragraph = format!("{}{}{}{}", N_UP, O_MORE, SPACE, bottles),
_ => paragraph = format!("{}{}{}", n_str, SPACE, bottles),
}
By using match
extensively, we also can remove the nested if-else
:
// Build closing
let phrase = match n {
0 => format!("{}{}{}", NINE_NINE, SPACE, bottles),
1 => format!("{}{}{}{}", N_DOWN, O_MORE, SPACE, bottles),
_ => format!("{}{}{}", qty, SPACE, bottles),
};
Finally, we get to such a result:
const BOTTLE: &str = "bottle";
const BOTTLES: &str = "bottles";
const OF_BEER: &str = "of beer";
const UPPERCASE_N: &str = "N";
const LOWERCASE_N: &str = "n";
const O_MORE: &str = "o more";
const ON_THE_WALL: &str = "on the wall";
const SPACE: &str = " ";
const COMMA: &str = ",";
const PERIOD: &str = ".";
const NL: &str = "\n";
const TAKE: &str = "Take";
const IT: &str = "it";
const ONE: &str = "one";
const DOWN_AND_PASS: &str = "down and pass it around";
const GO_STORE: &str = "Go to the store and buy some more";
const NINE_NINE: &str = "99";
pub fn verse(n: u32) -> String {
let mut paragraph: String = String::new();
// Is `bottle` plural in this paragraph?
let bottles: String = match n {
1 => BOTTLE.to_string(),
_ => BOTTLES.to_string(),
};
// Create first half of first verse.
let phrase: String = match n {
0 => UPPERCASE_N.to_owned() + O_MORE + SPACE + &bottles,
_ => n.to_string() + SPACE + &bottles,
};
paragraph = paragraph + &phrase;
paragraph = paragraph + SPACE + ON_THE_WALL + COMMA + SPACE;
// Create second half of first verse.
let phrase: String = match n {
0 => LOWERCASE_N.to_owned() + O_MORE,
_ => n.to_string(),
};
paragraph = paragraph + &phrase;
paragraph = paragraph + SPACE + &bottles + PERIOD + NL;
// Create first half of second verse.
let phrase: String = match n {
0 => GO_STORE.to_string(),
1 => TAKE.to_owned() + SPACE + IT + SPACE + DOWN_AND_PASS,
_ => TAKE.to_owned() + SPACE + ONE + SPACE + DOWN_AND_PASS,
};
paragraph = paragraph + &phrase;
// Add punctuation
paragraph = paragraph + COMMA + SPACE;
// Create second half of second verse.
let phrase = match n {
0 => NINE_NINE.to_owned() + SPACE + &bottles,
1 => LOWERCASE_N.to_owned() + O_MORE + SPACE + BOTTLES,
_ => n.to_string() + SPACE + &bottles,
};
paragraph = paragraph + &phrase;
// Finish second verse
paragraph = paragraph + SPACE + OF_BEER + SPACE + ON_THE_WALL + PERIOD + NL;
return paragraph;
}
fn main() {
let start: u32 = 10;
let stop: u32 = 0;
let mut song: String = String::new();
for x in (stop..start+1).rev() {
song = song + &verse(x);
}
println!("{}", song);
}
This is still not too clear code to me, and I'd rather work with the
first variant.
On a final note, there were places were you use n_str
or format!("{}", n)
, be mindful of this types of errors.
Likewise, typically start/stop
or begin/end
are used, but try not to mix them up.
Lastly, I had a lot of trouble distinguishing between O_DOWN
N_DOWN
, N_UP
.