Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix early exit from validation before all terms are used #3

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

Merged
maneatingape merged 1 commit into maneatingape:main from markjfisher:main
Dec 7, 2024

Conversation

Copy link
Contributor

@markjfisher markjfisher commented Dec 7, 2024

I'm learning rust, came across your repository and have used it as the basis of my own to learn from.
Thankyou so much for making it public, I'm learning a lot by it.

For Day07, I managed part 1, but part 2 broke me and I looked to your repo for inspiration.

However it didn't work on my Part1 data.
I compared the lines that were different and found that on 2 of my input lines, your code has a false positive.

11174: 15 8 9 79 74
729: 6 6 7 37 650

The valid function is returning a match before it has exhausted all terms. In both cases it finds that the target is reached before considering the first digit in the list (i.e. the final term isn't considered).

I've added a naive simple fix, you may want to make it more compact.

Again many thanks for your repo, it's been an inspiration, as I normally do this in Kotlin, but this year tried to pick up rust again (my second attempt in last few years).

Copy link
Owner

@maneatingape maneatingape left a comment
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the analysis and fix!

Was wondering if the input was always structured so that I could wing it using the test_value as a failure, now we know. 🙂

markjfisher reacted with thumbs up emoji
@maneatingape maneatingape merged commit cd1b76c into maneatingape:main Dec 7, 2024
Copy link
Owner

Your fix also improved the benchmark from 220 μs to 147 μs. Kudos!

Copy link
Owner

Updated my comment in the solution megathread to credit your fix.

You're the first contributor to this repo!

markjfisher reacted with thumbs up emoji markjfisher reacted with laugh emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maneatingape maneatingape maneatingape approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /