Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grains exercise: test against integer values as strings. #326

Merged
merged 2 commits into from
Nov 9, 2024

Conversation

glennj
Copy link
Contributor

@glennj glennj commented Nov 8, 2024

To work around Vimscripts 2^63 limit for integers, the tests have been created to expect the integral values as strings.

Following #322

has been created to expect the integral values as strings.
Copy link
Member

@kotp kotp left a comment

Choose a reason for hiding this comment

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

One change to not mistakenly suggest that it will only handle large numbers.

Otherwise, I think this is looking good!

exercises/practice/grains/.meta/example.vim Outdated Show resolved Hide resolved
@kotp kotp added x:module/practice-exercise Work on Practice Exercises x:action/create Work on something from scratch x:knowledge/intermediate Quite a bit of Exercism knowledge required x:status/claimed Someone is working on this issue x:type/content Work on content (e.g. exercises, concepts) x:rep/large Large amount of reputation labels Nov 8, 2024
@kotp kotp merged commit d0b1c14 into exercism:main Nov 9, 2024
3 checks passed
@kotp
Copy link
Member

kotp commented Nov 9, 2024

Thank you for this @glennj a lot of work when into this to not deviate too far from story, and I think it provides opportunity for learners to stretch their problem solving skills!

Copy link
Contributor

@blakelewis blakelewis left a comment

Choose a reason for hiding this comment

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

Small typo in instructions.append.md: "beyond the integer size limitation ov Vimscript." As a nitpick, I think that "limit" fits better than "limitation".

@blakelewis
Copy link
Contributor

Very nice solution. Thanks!

@kotp
Copy link
Member

kotp commented Nov 9, 2024

Small typo in instructions.append.md: "beyond the integer size limitation ov Vimscript." As a nitpick, I think that "limit" fits better than "limitation".

There is a typo, but it is likely the change to "of" from "ov".

The "limitation" word is correct, since is a language limitation, not an arbitrary limit.

@kotp
Copy link
Member

kotp commented Nov 9, 2024

@blakelewis I would accept a patch for this, if you want to correct the "of" word that should be here.

@blakelewis
Copy link
Contributor

Sorry, I had two comments. First, change "ov" to "of". Second, I think that the use of strings lets you go past the integer limit. I am not really sure what it means to go "beyond the limitation." One might interpret that as going to even greater limitations! But it is an arguable point and I won't press it.

I can make a patch to fix the typo later today

@kotp
Copy link
Member

kotp commented Nov 9, 2024

Sorry, I had two comments. First, change "ov" to "of".

This is the patch that I would accept for sure.

Second, I think that the use of strings lets you go past the integer limit. I am not really sure what it means to go "beyond the limitation." One might interpret that as going to even greater limitations! But it is an arguable point and I won't press it.

The limitation of Vimscript in terms of Integers is the reason for the example solution being what it is as a "prove we can solve the exercise as given."

You are, of course, right that we by pass the limitation by using strings, but we will hit another limitation in terms of strings, which we do not even hint here (and it is not important to address for this exercise).

I can make a patch to fix the typo later today

Sounds good. Usually we would discuss it in the forums first, but this patch is very small and discussed here instead, so it will be no problem.

@kotp kotp mentioned this pull request Nov 10, 2024
@glennj glennj deleted the grains-int-limit branch November 10, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/create Work on something from scratch x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:rep/large Large amount of reputation x:status/claimed Someone is working on this issue x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants