Skip to content

Add a Day type to provide better handling of day value #35

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
merged 17 commits into from
Nov 22, 2023

Conversation

tguichaoua
Copy link
Contributor

@tguichaoua tguichaoua commented Nov 22, 2023

Introduce a type Day to enforce static validity of the value and provide better error message to the user.

Before / after example with an invalid value
image

Compile time checked value
image

@tguichaoua tguichaoua marked this pull request as draft November 22, 2023 10:01
@fspoettel fspoettel self-requested a review November 22, 2023 10:17
@fspoettel
Copy link
Owner

Hey @tguichaoua, let me know once this is ready and I'll review it. The change looks like a good improvement, happy to merge it once ready. Thank you! 🙂

@tguichaoua tguichaoua marked this pull request as ready for review November 22, 2023 10:38
@tguichaoua
Copy link
Contributor Author

tguichaoua commented Nov 22, 2023

@fspoettel
it should be ready now 😉

Copy link
Owner

@fspoettel fspoettel left a comment

Choose a reason for hiding this comment

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

Looks good - the only real issue I found are that the tests in the scaffold template do not interpolateDAY_NUMBER yet, so they don't work.

@@ -12,7 +14,7 @@ pub fn part_two(input: &str) -> Option<u32> {
None
}

advent_of_code::main!(DAY);
advent_of_code::main!(DAY_NUMBER);

#[cfg(test)]
mod tests {
Copy link
Owner

Choose a reason for hiding this comment

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

The unit tests still have references to DAY which should be DAY_NUMBER now.

Copy link
Contributor Author

@tguichaoua tguichaoua Nov 22, 2023

Choose a reason for hiding this comment

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

advent_of_code::main! creates a const value DAY of type Day.
The DAY in unit tests refer to this const.

https://github.com/tguichaoua/advent-of-code-rust/blob/94c5a487cc59501bc60261d36a9ddd7e2567ff83/src/template/mod.rs#L26-L27

Copy link
Owner

Choose a reason for hiding this comment

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

Gotcha, missed that.

tguichaoua and others added 6 commits November 22, 2023 12:54
Co-authored-by: Felix Spöttel <[email protected]>
Co-authored-by: Felix Spöttel <[email protected]>
Co-authored-by: Felix Spöttel <[email protected]>
Co-authored-by: Felix Spöttel <[email protected]>
@fspoettel fspoettel merged commit 6d9bf34 into fspoettel:main Nov 22, 2023
@fspoettel
Copy link
Owner

Thank you for the contribution! 🙂

@tguichaoua tguichaoua deleted the day branch November 22, 2023 13:21
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