The Program Has a Bug
Its output begins:
On the first day of Christmas my true love sent to me
And a partridge in a pear tree.
On the second day of Christmas my true love sent to me
A partridge in a pear tree,
And two turtle doves.
On the third day of Christmas my true love sent to me
A partridge in a pear tree,
Two turtle doves,
And three French hens.
When it should be something close to:
On the first day of Christmas, my true love sent to me
A partridge in a pear tree.
On the second day of Christmas, my true love sent to me
Two turtledoves and
A partridge in a pear tree.
On the third day of Christmas, my true love sent to me
Three French hens,
Two turtledoves and
A partridge in a pear tree.
There are several equally-valid variations. This isn’t quite the one you wanted, but I encourage you to fix it so the first verse says “A partridge in a pear tree,” and later ones end in “And a partridge in a pear tree.” (This shouldn’t use a conditional branch on each iteration of a loop; see below.) Another variation you might try that lets you use sentence capitalization is “Four calling birds, three French hens, two turtledoves and a partridge in a pear tree,” all on a single line. Do what looks interesting to you!
Factor out Functions
Small functions, each doing one thing that’s easy to understand, are much nicer to work with. It also lets us move our variables out of main
or the global scope to where they’ll be used. It also lets this code be called from somewhere else in the program than main
.
For example, let’s say we’ve got a function that begins:
fn verse(n : usize) -> String {
/* Returns the n-th verse of “The Twelve Days of Christmas,” followed by
* a newline.
*/
The main
function then simplifies to
pub fn main() {
print!("{}", verse(1));
for i in 2..=DAYS_OF_CHRISTMAS {
print!("\n{}", verse(i));
}
}
There are other ways we could do it, but buffering each verse (or the output for each item in a batch) in a String
, printing that, and then dropping it is a good compromise between buffering nothing and buffering the entire output of the program before printing anything. You can still print!
every individual piece of the song, as you were doing, and this will eliminate all use of dynamic memory. So that isn’t a bad approach, just a different trade-off.
Use Constants where Appropriate.
In this toy program, it might seem silly to define
const DAYS_OF_CHRISTMAS : usize = 12;
But let’s say you’ve got a program that could also do “The Twelve Joys of Mary” or “Thirty Days Hath September,” with the data for twelve months. Or anything else where the constant 12 has some other meaning. It now becomes very difficult to go through the program and update all of the instances of 12
that are supposed to be the number of verses in this song, and not any other 12
that appears in the source. Or to catch a typo like 21
.
In this case, it’s not totally implausible that the type of this constant might change, either. In this case, I picked usize
because I kept your great idea of storing the data for each verse in an array, and in that case, its index should have type usize
. But this is a small number that would fit in an i8
, so a smaller type might be more convenient to work with.
It’s also good practice to write constant functions and data when you can, because those optimize better and are more robust against bugs that modify program state, and have simple static
lifetimes. Combining this and the last piece of advice, we can write this little function to return first
, second
, third
, etc. as a &str
:
const fn ordinal(n : usize) -> &'static str {
/* Returns an uncapitalized English ordinal number between 0 and 12.
*/
const ORDINALS : [&str; DAYS_OF_CHRISTMAS+1] = [
"zeroth",
"first",
"second",
"third",
"fourth",
"fifth",
"sixth",
"seventh",
"eighth",
"ninth",
"tenth",
"eleventh",
"twelfth"
];
assert!(n < ORDINALS.len(), "Ordinal out of range."); // Always check for buffer overruns!
return ORDINALS[n];
}
This is pretty similar to what you did, but now the ordinal string constants are only in scope for the one part of the program that needs to see them. You could also write a more general-purpose ordinal function that could be used for other purposes and call that. (Although, if it were fully general, able to return values like "one billion two hundred thirty-four million five hundred sixty-seven thousand eight hundred ninetieth"
, it would no longer be const
because you would then be generating the result string dynamically.)
Print the Lines of Each Verse in the Right Order
There’s a good example of how to do this right above where you were reading, in Section 3.5 of the Rust Book.
for number in (1..4).rev()
A for
loop iterates on an iterator, and in this case, within each verse, you want to iterate in reverse. So, where you now have
for gift_no in 0..(verse_no+1) {
This should be something like
for gift_no in (0..=verse_no).rev() {
But see below:
Make the Control Flow Work for You
Currently, you use std::cmp::Ordering;
and then match
every verse to see if it is the first. This is unnecessarily complex. You know that this condition will be false for every repetition but the last, which will always be the gift with index 0
.
So you could actually just do your for
loop from verse_no
to 1
, and then print the final line after the loop. This eliminates the unnecessary extra match
on each iteration.
Similarly, you currently test if verse_no != days.len()-1
on each iteration, duplicating the check at the top of the loop, to determine whether to print a second newline after each verses. I solve this, above, by printing the first verse separately, and then printing verses 2 through 12 with an extra newline before them. Or you might also print verses 1 through eleven in a loop, followed by an extra newline, and then the final verse on its own.
Putting it All Together
This example is simplified compared to your version. That leaves you some space to tweak it in the ways you wanted, such as making the last line of each verse properly display either “A partridge in a pear tree.” or “And a partridge in a pear tree.”
const DAYS_OF_CHRISTMAS : usize = 12;
const fn ordinal(n : usize) -> &'static str {
/* Returns an uncapitalized English ordinal number between 0 and 12.
*/
const ORDINALS : [&str; DAYS_OF_CHRISTMAS+1] = [
"zeroth",
"first",
"second",
"third",
"fourth",
"fifth",
"sixth",
"seventh",
"eighth",
"ninth",
"tenth",
"eleventh",
"twelfth"
];
assert!(n < ORDINALS.len(), "Ordinal out of range."); // Always check for buffer overruns!
return ORDINALS[n];
}
fn verse(n : usize) -> String {
/* Returns the n-th verse of “The Twelve Days of Christmas,” followed by
* a newline after.
*/
assert!( n > 0 && n <= DAYS_OF_CHRISTMAS,
"There are only twelve days of Christmas." );
const GIFTS : [&str; DAYS_OF_CHRISTMAS] = [
"A partridge in a pear tree.",
"Two turtledoves and",
"Three French hens,",
"Four calling birds,",
"Five golden rings,",
"Six geese a-laying,",
"Seven swans a-swimming,",
"Eight maids a-milking,",
"Nine ladies dancing,",
"Ten lords a-leaping",
"Eleven pipers piping,",
"Twelve drummers drumming,"
];
let mut result = String::from("On the ") +
ordinal(n) +
" day of Christmas, my true love sent to me\n";
for i in (0..n).rev() {
result += GIFTS[i];
result += "\n";
}
return result;
}
pub fn main() {
print!("{}", verse(1));
for i in 2..=DAYS_OF_CHRISTMAS {
print!("\n{}", verse(i));
}
}
Possible Improvements
There are other, more advanced variations you could try on verses
, such as replacing the loop in verses
with a collect_into
or fold
operation on the iterator. Another way to write verses
in a functional style (eliminating all mutable variables and side-effects), for example, would be:
fn verse(n : usize) -> String {
/* Returns the n-th verse of “The Twelve Days of Christmas,” followed by
* a newline after.
*/
assert!( n > 0 && n <= DAYS_OF_CHRISTMAS,
"There are only twelve days of Christmas." );
const GIFTS : [&str; DAYS_OF_CHRISTMAS] = [
"A partridge in a pear tree.",
"Two turtledoves and",
"Three French hens,",
"Four calling birds,",
"Five golden rings,",
"Six geese a-laying,",
"Seven swans a-swimming,",
"Eight maids a-milking,",
"Nine ladies dancing,",
"Ten lords a-leaping",
"Eleven pipers piping,",
"Twelve drummers drumming,"
];
let prefix = String::from("On the ") +
ordinal(n) +
" day of Christmas, my true love sent to me\n";
return (0..n).rev()
.fold( prefix, |s, i| s + GIFTS[i] + "\n" );
}
There’s another useful optimization you could make. Because the entire song is composed of twenty-seven string constant building blocks, and the verses all put them together in different ways, you could have each verse collect references to the chunks in a Vec<&'static str>
. This would still use a little bit of dynamic memory, but it would save you from making a deep copy of the entire contents of the song.
const DAYS_OF_CHRISTMAS : usize = 12;
const fn ordinal(n : usize) -> &'static str {
/* Returns an uncapitalized English ordinal number between 0 and 12.
*/
const ORDINALS : [&str; DAYS_OF_CHRISTMAS+1] = [
"zeroth",
"first",
"second",
"third",
"fourth",
"fifth",
"sixth",
"seventh",
"eighth",
"ninth",
"tenth",
"eleventh",
"twelfth"
];
assert!(n < ORDINALS.len(), "Ordinal out of range."); // Always check for buffer overruns!
return ORDINALS[n];
}
fn verse(n : usize) -> Vec<&'static str> {
/* Returns the n-th verse of “The Twelve Days of Christmas,” in the form of
* a collection of references to the building-blocks of the song.
*/
assert!( n > 0 && n <= DAYS_OF_CHRISTMAS,
"There are only twelve days of Christmas." );
const GIFTS : [&str; DAYS_OF_CHRISTMAS] = [
"A partridge in a pear tree.\n",
"Two turtledoves and\n",
"Three French hens,\n",
"Four calling birds,\n",
"Five golden rings,\n",
"Six geese a-laying,\n",
"Seven swans a-swimming,\n",
"Eight maids a-milking,\n",
"Nine ladies dancing,\n",
"Ten lords a-leaping\n",
"Eleven pipers piping,\n",
"Twelve drummers drumming,\n"
];
let mut result = Vec::from([
"On the ", ordinal(n), " day of Christmas, my true love gave to me\n"
]);
for i in (0..n).rev() {
result.push(GIFTS[i]);
}
return result;
}
pub fn main() {
for chunk in verse(1) {
print!( "{}", chunk );
}
for i in 2..=DAYS_OF_CHRISTMAS {
print!("\n");
for chunk in verse(i) {
print!( "{}", chunk );
}
}
}
```
"four calling birds"
. Also, what is so complicated about.to_uppercase()
or.to_lowercase()
? \$\endgroup\$chars
on a String and then extract the first letter (easy,.nth(0)
), call to_lowercase on this letter, extract anything except for the first letter (idk how to do this atm), then join both parts again (easy,format!()
). \$\endgroup\$