[48in24 Exercise] [02-06] Roman Numerals

[kahgoh]

Just opened the PR to add the exercise for LFE: Add Roman numerals practice exercise by kahgoh · Pull Request #188 · exercism/lfe · GitHub

[mpg]

Hi! I was just doing this exercise as part of the challenge (started a bit late), and noticed a test gap: the longest possible output (3888 → MMMDCCCLXXXVIII) is not tested. This is important for languages like C where you have to allocate a large enough buffer, so IMO is should be tested. (Though scrolling through community solutions it looks like most got it right.)

I’m not familiar enough with how exercism works to open a PR adding such a test (especially as I suspect tests for individual languages are somehow generated from a list shared between all the tracks that have this exercise?) so I thought I’d mention it here hoping that someone could add this test.

Thanks in advance!

Edited to add: the Python “dig deeper” page says “the longest string needed to represent a Roman numeral is 14 characters (MMDCCCLXXXVIII)” but I think it should be “15 characters (MMMDCCCLXXXVIII)”.

[colinleach]

@mpg

the Python “dig deeper” page says “the longest string needed to represent a Roman numeral is 14 characters (MMDCCCLXXXVIII)” but I think it should be “15 characters (MMMDCCCLXXXVIII)”.

Yes, that looks like my mistake. Would you like to submit a PR for this? If not, I’m happy to correct it.

[BNAndras]

@mpg

I’m not familiar enough with how exercism works to open a PR adding such a test

problem-specifications/exercises at main · exercism/problem-specifications · GitHub contains the canonical data from which tracks should source their tests when porting a specific practice exercise like roman-numerals. Contributing there means more tracks can see and discuss your proposed edits, but some tracks also implement their own tests alongside the canonical ones. What you’re proposing might be more appropriate as a track-specific test since the length of the returned string wouldn’t be an issue on any of the tracks I maintain.

[IsaacG]

Community | Exercism's Docs

[mpg]

@colinleach

Yes, that looks like my mistake. Would you like to submit a PR for this? If not, I’m happy to correct it.

I raised a PR then!

[mpg]

@BNAndras

problem-specifications/exercises at main · exercism/problem-specifications · GitHub contains the canonical data from which tracks should source their tests when porting a specific practice exercise like roman-numerals.

Thanks for the pointer! That’s the main thing I was missing :)

@BNAndras

Contributing there means more tracks can see and discuss your proposed edits, but some tracks also implement their own tests alongside the canonical ones. What you’re proposing might be more appropriate as a track-specific test since the length of the returned string wouldn’t be an issue on any of the tracks I maintain.

In doubt, I’ve created a PR on problem-specifications with the following reasoning:

1. If/when this exercise is added to a language with manual memory management in the future, that way it’ll automatically get that test.
2. The documentation (thanks @IsaacG for the link!) seems to encourage that.

However if it turns out that was misguided and this PR is rejected, I’m happy to raise a PR on the C repo instead.

[BNAndras]

@mpg

doubt, I’ve created a PR on problem-specifications

PR merged.

[BethanyG]

PR Merged.