Skip to content

Armstrong Numbers with big-integers - #1155

Merged
colinleach merged 3 commits into
exercism:mainfrom
colinleach:armstrong-bigint
Oct 24, 2026
Merged

colinleach merged 3 commits into
exercism:mainfrom
colinleach:armstrong-bigint

Conversation

@colinleach

Copy link
Copy Markdown
Contributor

I think this gets us to where #1152 fell disappointingly short. It runs correctly in Docker. Admittedly, not at my first attempt.

@github-actions

Copy link
Copy Markdown
Contributor

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

instructions.append.md omits how to enable the opt-in tests and uses misleading/comma-splice wording, deviating from the convention followed by sibling bonus-test exercises (reverse-string, complex-numbers).

1 open issue
What changed in this PR

This PR enables the two previously-excluded big-integer canonical tests for the armstrong-numbers practice exercise, addressing the gap left by #1152. The tests are made opt-in (gated behind @isdefined(TEST_BIG_INTEGERS)) so the ~578 existing student solutions are not broken, while students who want the challenge can enable them. The example solution is updated to compute digit powers in the number's own type so 128-bit integers suffice without BigInt.

Changes:

  • Un-exclude the two big-integer cases in tests.toml and emit them inside a TEST_BIG_INTEGERS-gated big_integers testset (template + generated runtests.jl).
  • Update .meta/example.jl to use digits(typeof(n), n) so powers are computed in typeof(n) (Int128 for the big literals), avoiding Int64 overflow.
  • Add a stub comment and .docs/instructions.append.md describing the optional bonus.
File Description
.meta/​tests.toml Removes include = false from the two big-integer cases so they are generated.
.meta/​template.j2 Splits cases into the normal loop and a @isdefined(TEST_BIG_INTEGERS)-gated bonus loop.
runtests.jl Regenerated output containing the opt-in big_integers testset.
.meta/​example.jl Reworked solution using digits(typeof(n), n) to avoid overflow; sets the enabling const.
armstrong-numbers.jl Stub gains a commented-out line to enable the optional tests.
.docs/​instructions.append.md New bonus note stating 128-bit integers suffice.

I verified the correctness claim: both literals (186...976990 and 115...522401) parse as Int128, and every intermediate k^length(d) term and the accumulated sum stay within Int128 max, so the "no need for BigInt" statement holds for the example solution.

Review effort: Balanced

Comment thread exercises/practice/armstrong-numbers/.docs/instructions.append.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@colinleach
colinleach requested a review from depial October 23, 2026 23:48

@depial depial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I'd assume it's the extra info in the append.md throwing the translation issue, but I still haven't read too much into that whole thing yet.

@colinleach colinleach added the ready-to-translate The English in this PR is final; queue its translations label Oct 24, 2026
@colinleach

Copy link
Copy Markdown
Contributor Author

The translation thing: once everything is finalized, we add the ready-to-translate label to trigger translation. It costs Exercism $$, so wait until there will be no further changes that require re-translation.

Within 10 minutes (usually), we get a "translated" message and can merge.

It adds a bit of delay, but it seems a price worth paying to bring in more non-anglphone students.

exercism-i18n Bot added a commit to exercism/i18n that referenced this pull request Oct 24, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@exercism-i18n

exercism-i18n Bot commented Oct 24, 2026

Copy link
Copy Markdown

This PR has been translated 🚀

@colinleach
colinleach merged commit 12a17b4 into exercism:main Oct 24, 2026
12 of 13 checks passed
@colinleach colinleach mentioned this pull request Oct 24, 2026
82 tasks done
@colinleach
colinleach deleted the armstrong-bigint branch October 25, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-translate The English in this PR is final; queue its translations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants