Skip to content

Ch 21.02 - listings missing required code#4350

Open
rickrain wants to merge 7 commits intorust-lang:mainfrom
rickrain:rickrain/ch_21_02
Open

Ch 21.02 - listings missing required code#4350
rickrain wants to merge 7 commits intorust-lang:mainfrom
rickrain:rickrain/ch_21_02

Conversation

@rickrain
Copy link
Copy Markdown
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
Copy Markdown
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