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

feat: add smart pointer concept #824

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Conversation

vaeng
Copy link
Contributor

@vaeng vaeng commented Mar 1, 2024

This is long, but I think it is pretty straightforward. Maybe I have tunnel vision and this is not readable at all :D

@vaeng vaeng added x:action/proofread Proofread text x:type/docs Work on Documentation x:module/concept Work on Concepts x:rep/large Large amount of reputation labels Mar 1, 2024
@vaeng
Copy link
Contributor Author

vaeng commented Mar 1, 2024

About.md will follow when the main text has been reviewed.

Copy link
Contributor

@siebenschlaefer siebenschlaefer left a comment

Choose a reason for hiding this comment

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

Not bad!

This description focuses on the deallocation. But IMHO an equally important aspect is that the objects gets properly destructed.

A large part of this PR describes weak pointers. Did you consider omitting them? For somebody who just learns about smart pointers std::unique_ptr is the most important thing, std::shared_ptr is a clear second, but std::weak_ptr is IMHO far less important. Dedicating a large portion of this document to std::weak_ptr does not reflect that.

This PR describes the motivation and technical aspects of the three smart pointers. You could consider adding some advice: When raw pointers are fine to use (if they are not owning), which smart pointer should be used when (by default: std::unique_ptr).

concepts/smart-pointers/.meta/config.json Outdated Show resolved Hide resolved
concepts/smart-pointers/.meta/config.json Outdated Show resolved Hide resolved
concepts/smart-pointers/introduction.md Outdated Show resolved Hide resolved
concepts/smart-pointers/introduction.md Outdated Show resolved Hide resolved
concepts/smart-pointers/introduction.md Outdated Show resolved Hide resolved
concepts/smart-pointers/introduction.md Outdated Show resolved Hide resolved
concepts/smart-pointers/introduction.md Outdated Show resolved Hide resolved
@vaeng vaeng requested a review from siebenschlaefer March 12, 2024 13:22
Copy link
Contributor

@siebenschlaefer siebenschlaefer left a comment

Choose a reason for hiding this comment

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

I like it, I don't even have anything to criticize.
The only think I'm missing is the reference to works of fiction.

@vaeng
Copy link
Contributor Author

vaeng commented Mar 20, 2024

The only think I'm missing is the reference to works of fiction.

Apart from king Arthur there is also "The Expanse" in there.

@vaeng vaeng merged commit 6bcfce7 into main Mar 20, 2024
8 checks passed
@vaeng vaeng deleted the feat--add-smart-pointer-concept branch March 20, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/proofread Proofread text x:module/concept Work on Concepts x:rep/large Large amount of reputation x:type/docs Work on Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants