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

enums3.rs Comment confusing for new users. #1706

Closed
Acivo opened this issue Oct 6, 2023 · 3 comments
Closed

enums3.rs Comment confusing for new users. #1706

Acivo opened this issue Oct 6, 2023 · 3 comments

Comments

@Acivo
Copy link

Acivo commented Oct 6, 2023

The comment in the enums3.rs exercise:

// TODO: create a match expression to process the different message
// variants
// Remember: When passing a tuple as a function argument, you'll need
// extra parentheses: fn function((t, u, p, l, e))

Was very confusing to me, when I was doing the exercise. It made me second guess my attempt (which turned out to be correct), because it made me think that I needed to have extra parentheses in this line:

Message::ChangeColor((r, g, b)) => self.color = (r, g, b),

After asking in Discord, I now understand that it was intended to remind me of the need for extra parentheses in a function signature, for future reference, but as it wasn't actually applicable at all in the exercise, I found myself searching for a place that I needed to place those extra parentheses.

@CheaterCodes
Copy link

For reference:
I believe this was first discussed here: 4b6540c
And then later introduced in the pull request here:
#1310

I'm still not sure why the comment was added, but if the point is to demonstrate the occasional need for double parentheses, maybe an enums4.rs would be a better solution.

@CheaterCodes
Copy link

After having re-read, it appears that enums3.rs expects you to create a message like this, but that change was then later reversed or never applied:

enum Message {
  ChangeColor((u8, u8, u8)),
  ...
}

However, it is never indicated in the usage that this is the desired use. Maybe something like this should be added to the use case:

let color = (255, 0, 255);
state.process(Message::ChangeColor(color));

Putting the variable on a separate line makes it clear that the double parentheses added in the above commit are intentional.

@mo8it
Copy link
Contributor

mo8it commented Jul 7, 2024

Fixed in 708cfef by avoiding this confusion with double parentheses.

@mo8it mo8it closed this as completed Jul 7, 2024
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

No branches or pull requests

3 participants