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

Get rid of “new” #868

Open
tylerjw opened this issue Dec 1, 2021 · 3 comments · Fixed by #2019
Open

Get rid of “new” #868

tylerjw opened this issue Dec 1, 2021 · 3 comments · Fixed by #2019
Labels
good first issue Good for newcomers

Comments

@tylerjw
Copy link
Member

tylerjw commented Dec 1, 2021

https://github.com/cpp-best-practices/cppbestpractices/blob/master/08-Considering_Performance.md#get-rid-of-new

@tylerjw tylerjw added the good first issue Good for newcomers label Dec 1, 2021
@Robotawi
Copy link

@v4hn
Copy link
Contributor

v4hn commented May 31, 2022

The examples you cite are obviously bad code either way.
The first one uses new multiple times(?) for a temporary structure (because it would be/was back in the days too much for the stack?).
If the function has to be reentrant that can make sense, but even then the style is horrible and should use one unique_ptr to a single allocated block instead.

The second one could probably be replaced by a local JointConstraint and a emplace_back(std::move(...)) operation.
But it is bad design to begin with to have multiple lists that should be consistent all hold shared_ptr ownership to the same data.

It seems to me cleaning up the (/some of the) remaining new operators might be best done by reorganizing things in the datastructures themselves instead of replacing them by simple make_*() calls, so it might be better to look at a few of them only and think about how this can be avoided better by adapting the environment of the code.

@TanmayAgarwal123
Copy link

I would love to work on this project, can you assign me
Regards,
Tanmay Agarwal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants