"Say" instructions are inconsistent with test

The instruction of the Say exercise start with:

Given a number from 0 to 999,999,999,999, spell out that number in English.

But the tests are testing numbers that are much bigger than that, such as Max i64 and Max u64.

The instructions also say:

Some good test cases for this program are:
(…) -1

whereas the function signature takes a u64.

I sent a PR: Update instructions to match tests and function signature. by ColinPitrat · Pull Request #2089 · exercism/rust · GitHub

The instructions come from the upstream problem-specifications repo that all 77+ Exercism tracks pull from. As a result, you shouldn’t update the instructions or exercise config directly since those changes might get overwritten the next time the files are synced.

An instructions append file would be the preferred route to introduce track-specific instructions.

Edit - More info about an append is at Practice Exercises | Exercism's Docs. However, wait until a Rust maintainer indicates this is the way they want you to proceed.

(Note I am not a Rust maintainer.)

See Practice Exercises | Exercism's Docs

.docs/instructions.md

If the exercise implements a Problem Specifications Exercise, this file’s contents should match the Problem Specification Exercise’s instructions.md file (or description.md file if there is no instructions.md file). configlet has functionality to automatically sync the contents of this file.

We should not make track (language) specific changes in this file.

Note that configlet also syncs the blurb in .meta/config.json, we should not change that either.

.docs/introduction.append.md

If the track maintainers decide additional track-specific instructions are required, they are placed in this optional file.

For an example, see hello-world

Rust maintainer here, adding an instructions.append.md file seems reasonable to me. PRs welcome.

2 Likes

I do think Colin has a point that -1 cannot appear if it’s an unsigned integer. In other languages, of course the instructions…

Your program should complain loudly if given a number outside the blessed range.

…would be fine.

In this case, I do think that it’s fine for rust. It should probably panic if you try to enter -1.

I added this information on the PR (for visibility). Feel free to dismiss my review and/or close again.

Are you suggesting we change the exercise design to accept signed integers?

It’s worth a discussion I think!

A lot of these problem specs are old and didn’t consider these things but we have quite a few maintainers around to help make decisions about this :grin:

Incidentally, I started a thread about updating the instructions the other day at Update confusing instructions for `say`.

I feel like you should keep the input an unsigned int given the exercise clearly indicates the number should be 0 to 999,999,999,999 so a negative number wouldn’t make a lot of sense here. That aligns with Rust’s Grains implementation which also takes an unsigned int since you can’t conceptually have an negative numbered square on the chessboard.

2 Likes

Yes, we have a lot of exercises where the function signatures are quite strict, which prevents many errors at compile time. This is idiomatic Rust. So I think it’s not worth it to loosen the function signature just to match the introduction text a little better.

Maybe the introduction could be made a little more vague upstream. So it hints that there may be a test involving -1, but there doesn’t have to be.

3 Likes

I think this is a good point. You can now link to it in the future if someone brings it up. I agree with this assessment in the same way that in JS and TS we don’t write a gazillion defensive input validation algos for each function.

Yeah, I could be in support of that change.

There’s already an instructions.append.md and indeed, it does display at the end of the instructions:

Extension
Add capability of converting up to the max value for u64: 18_446_744_073_709_551_615.
For hints at the output this should have, look at the last test case.

Somehow I missed it. So no further change is needed I guess.