Skip to content

Comments

Add new exercise: prism#339

Merged
glennj merged 5 commits intoexercism:mainfrom
IsaacG:prism
Feb 22, 2026
Merged

Add new exercise: prism#339
glennj merged 5 commits intoexercism:mainfrom
IsaacG:prism

Conversation

@IsaacG
Copy link
Member

@IsaacG IsaacG commented Feb 16, 2026

No description provided.

@IsaacG IsaacG added x:size/large Large amount of work x:module/practice-exercise Work on Practice Exercises x:rep/large Large amount of reputation x:action/create Work on something from scratch labels Feb 16, 2026
@IsaacG IsaacG requested a review from glennj February 17, 2026 06:07
@@ -0,0 +1,8 @@
# The first line of input is the starting x, y and angle.
# Any remaining input is the prisms: x, y, angle and ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put ID as the first field please?

Suggested change
# Any remaining input is the prisms: x, y, angle and ID.
# Any remaining input is the prisms: ID, x, y, and refraction angle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 37 to 39
0 -10 0 3
-10 0 0 1
10 0 0 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why these aren't sorted by ID?

Copy link
Member Author

Choose a reason for hiding this comment

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

This matches the problem spec. I believe there was a misunderstanding in that PR that resulted in randomized prism order here. We can correct it but it should probably be done in the problem spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't matter how they appear in the test file, the code should find the next point on the path. I don't think we need to slavishly follow the canonical data for this, let's make it easier to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack. Added sort_by(.id).

@glennj glennj merged commit 438bc2b into exercism:main Feb 22, 2026
2 checks passed
@glennj
Copy link
Contributor

glennj commented Feb 22, 2026

Thanks Isaac!

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:module/practice-exercise Work on Practice Exercises x:rep/large Large amount of reputation x:size/large Large amount of work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants