Skip to content

Ch 21.02 - listings missing required code #4350

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

rickrain
Copy link
Contributor

@rickrain rickrain commented May 5, 2025

This PR updates listings 21-17 and 21-18 to show changes that were necessary to the Worker struct.

In listing 21-17, unless you also update the type for the Worker::thread field, you will get compiler errors about mismatched types, in addition to, the ownership errors that were expected. Since these additional errors were not mentioned in the text, the reader should see that the struct needed updating too. It's clear from the text that the intent was to focus on the ownership/borrowing errors - this change accomplishes that.

Note: I also updated the markdown text in ch21-02-multithreaded.md to explain the necessary change in the struct.

Similarly, in listing 21-18, the type for the Worker::thread field needs to be updated with the new smart pointer declaration, otherwise it won't compile. I didn't add any text to the markdown for this change, since the explanation from 21-17 is already understood.

@rickrain
Copy link
Contributor Author

rickrain commented May 5, 2025

It seems that the CI is failing because my implementation didn't have the curly brackets in my closure and instead just processed receiver as the expression. So, instead of what the text shows

        let thread = thread::spawn(|| {
            receiver;
        });

my code was

        let thread = thread::spawn(|| 
            receiver
        );

This seemed to have happened when I ran cargo fmt. Not sure what the maintainers will prefer. I believe you should be able to format your code without it changing required type declarations. Curious how others feel about it.

Oddly, when I run cargo fmt against the listing project, it doesn't do this. Not sure why my project behaves differently. Perhaps this PR is just noise. 😄

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