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

Secret Handshake approach #1636

Merged
merged 13 commits into from
Mar 3, 2023

Conversation

bobahop
Copy link
Member

@bobahop bobahop commented Mar 2, 2023

An approach for Secret Handshake.

@bobahop
Copy link
Member Author

bobahop commented Mar 2, 2023

@ErikSchierboom: Rust approach for Secret Handshake ready for review.

Comment on lines +7 to +24
pub fn actions(n: u8) -> Vec<&'static str> {
let (mut action, action_incr, end) = match n & REVERSE_SIGNS {
0 => (0, 1, 4),
_ => (3, -1, -1),
};
let mut output: Vec<&'static str> = Vec::new();

loop {
if action == end {
break;
}
if (n & (1 << action)) != 0 {
output.push(SIGNS[action as usize])
}
action += action_incr
}
output
}
Copy link
Member

@ErikSchierboom ErikSchierboom Mar 3, 2023

Choose a reason for hiding this comment

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

I must admit that I'm not well-versed in Rust, but to me this is very hard to read. It seems to almost obscure the logic. Is this an idiomatic way to solve the exercise in Rust? What about something simple like: https://exercism.org/tracks/rust/exercises/secret-handshake/solutions/aciba90?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are simpler approaches which don't offer as much in pedagogic value. Since I only did one approach, I used one which is not exactly unidiomatic, but which afforded more in the way of explanation.

Copy link
Member

Choose a reason for hiding this comment

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

Right. Okay!

Copy link
Member

Choose a reason for hiding this comment

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

It is an interesting approach

@ErikSchierboom ErikSchierboom merged commit 2b717ef into exercism:main Mar 3, 2023
dem4ron pushed a commit to dem4ron/rust-exercism that referenced this pull request Mar 9, 2023
Add approach to `secret-handshake`
dem4ron pushed a commit to dem4ron/rust-exercism that referenced this pull request Mar 9, 2023
Add approach to `secret-handshake`
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