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

feat: add high-scores exercise #176

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

keiravillekode
Copy link
Contributor

The last four canonical test cases for this exercise check that calling one method doesn't stop another method from returning the correct result.

These test cases are the reason I used (mut high_scores HighScores) - without the mut, those test cases would be largely redundant.

Comment on lines 3 to 5
struct HighScores {
scores []int
}
Copy link
Contributor

Choose a reason for hiding this comment

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

question: is there a particular reason to do this as a struct instead of just as an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just tried

type HighScores = []int

and experienced compiler crashes:

https://github.com/keiravillekode/vlang/blob/b36ed1eea7ca15d748a3c3d43d6376be0b778a69/exercises/practice/high-scores/.meta/example.v

---- Testing... -------------------------------------------------------------------------------------------------
-----------------------------------------------------------------------------------------------------------------
==================                                                                                               
/tmp/v_1000/tsession_7ff928207740_42009345629388/run_test.6178438000952218471.tmp.c:22171: error: cannot convert 'struct array *' to 'struct array'
...
==================
(Use `v -cg` to print the entire error message)

builder error: 
==================
C error. This should never happen.

This is a compiler bug, please report it using `v bug file.v`.

https://github.com/vlang/v/issues/new/choose

You can also use #help on Discord: https://discord.gg/vlang

I'll check if this happens with the latest compiler, and if so report a compiler bug.

Even if this approach works, I think high_scores.v should still use struct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I tried the type HighScores = []int example solution with a recent V compiler, there were no compiler crashes.

PR to update test runner: exercism/vlang-test-runner#39

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a current compiler crash if we make a small change to the personal_top_three() example solution.

See vlang/v#20075

The crash is avoided in the example solution uploaded for this PR.

Copy link
Contributor Author

@keiravillekode keiravillekode Dec 5, 2023

Choose a reason for hiding this comment

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

The compiler bug has been fixed. :-)

I expect the fix will be in weekly.2023.49 weekly.2023.50

Copy link
Contributor

Choose a reason for hiding this comment

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

Great! Let's wait until the compiler update and once your exercism/vlang-test-runner#39 gets merged in, we can merge this one in too

High scores example solution uses int array
instead of a struct, as the HighScores type.
Copy link
Contributor

@1ethanhansen 1ethanhansen left a comment

Choose a reason for hiding this comment

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

giving approval now, will merge in later (see previous comment thread)

@keiravillekode
Copy link
Contributor Author

exercism/vlang-test-runner#39 is ready to be merged, then this can be merged.

@1ethanhansen
Copy link
Contributor

Thanks so much!

@1ethanhansen 1ethanhansen merged commit cd39532 into exercism:main Jul 1, 2024
4 checks passed
@keiravillekode keiravillekode deleted the high-scores branch July 1, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants