1
\$\begingroup\$

I'm trying to learn Rust and for this reason I'm reading the Rust Book. It contains the following exercise:

Print the lyrics to the Christmas carol “The Twelve Days of Christmas,” taking advantage of the repetition in the song.

Here's my take. Note I'm only using material from chapters 1-3 of this book (since I've only read so far yet), with the exception of unreachable, which I nonetheless think needed to solve this exercise.

Sorry for managing the letter case in such an ugly way. Unfortunately, my Googling revealed that letter case in Rust is somewhat complicated and probably uses stuff from later chapters of the book, so I thought I'd "cheat" my way out of this problem like I did in this code:

use std::cmp::Ordering;

fn main() {
    let days = [
        ("first", "A", "a", " partridge in a pear tree"),
        ("second", "T", "t", "wo turtle doves"),
        ("third", "T", "t", "hree French hens"),
        ("fourth", "F", "f", "our calling birds"),
        ("fifth", "F", "f", "ive gold rings"),
        ("sixth", "S", "s", "ix geese a-laying"),
        ("seventh", "S", "s", "even swans a-swimming"),
        ("eighth", "E", "e", "ight maids a-milking"),
        ("ninth", "N", "n", "ine ladies dancing"),
        ("tenth", "T", "t", "en lords a-leaping"),
        ("eleventh", "E", "e", "leven pipers piping"),
        ("twelfth", "T", "t", "welve drummers drumming")
    ];
    
    for verse_no in 0..days.len() {
        let (day, _, _, _) = days[verse_no];
        println!("On the {day} day of Christmas my true love sent to me");
        
        for gift_no in 0..(verse_no+1) {
            let (_, upper, lower, rest) = days[gift_no];
            match gift_no.cmp(&verse_no) {
                Ordering::Less => println!("{upper}{rest},"),
                Ordering::Equal => println!("And {lower}{rest}."),
                Ordering::Greater => unreachable!(),
            }
        }
        if verse_no != days.len()-1 {
            println!("");
        }
    }
}

Comments?

\$\endgroup\$
5
  • \$\begingroup\$ There is a bug: "four calling birds". Also, what is so complicated about .to_uppercase() or .to_lowercase()? \$\endgroup\$
    – BCdotWEB
    Commented Dec 27, 2022 at 11:19
  • \$\begingroup\$ @BCdotWEB This is what I found: stackoverflow.com/questions/38406793/… \$\endgroup\$ Commented Dec 27, 2022 at 15:37
  • \$\begingroup\$ docs.rs/substring/latest/substring + to_uppercase()/to_lowercase() + stackoverflow.com/a/30154791/648075 etc. Plus: fix the bug. Your current code has an obvious bug and is thus not ready for code review. Did you even test it? \$\endgroup\$
    – BCdotWEB
    Commented Dec 27, 2022 at 16:16
  • \$\begingroup\$ @BCdotWEB (a) Yes, I did test my code, however it is not trivial to spot a repeated letter. It is easy to overlook such things. (b) I've fixed it here. (c) Thank you for suggesting me to use the Substring crate, however I think I'll pass on this. I checked it out and it doesn't really seem to be a well known or widely used crate. Maybe I'm overreacting, but since from time to time it becomes known that people upload malicious code to repositories such as NPM or Pip, it might be a good hygiene to only use well known, widely used libs. (con't) \$\endgroup\$ Commented Dec 30, 2022 at 1:36
  • \$\begingroup\$ Sooner or later I'm going to read the relevant chapters about strings and iterators in the Book and then I'll try to do this properly. My understanding is that I need to call 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\$ Commented Dec 30, 2022 at 1:38

1 Answer 1

1
\$\begingroup\$

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 );
      }
    }
}
```
\$\endgroup\$
2
  • \$\begingroup\$ Good advice... but strange commenting style. Typically comments describing an item appear before the item they document -- as doc-comments are -- and not within the item body. \$\endgroup\$ Commented Dec 31, 2022 at 17:17
  • \$\begingroup\$ @MatthieuM. Glad you liked the advice! I’m used to putting comments describing a function between the function declaration and the body. But the usual style for Rust is to put the opening brace on the same line as the function declaration. When I habitually put my comment in the same place I normally do, that meant it ended up inside the opening brace. I’ll reconsider next time. \$\endgroup\$
    – Davislor
    Commented Dec 31, 2022 at 17:36

Your Answer

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

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