-
-
Notifications
You must be signed in to change notification settings - Fork 510
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
Make use of smart pointers and references #316
Conversation
Replace `Device *PluginImplBase::parent` => `Device &PluginImplBase::parent`. And replaces everywhere as required.
Replaced wherever applicabale.
Smart pointers provide automatic resource management. In other words, they implement "Resource Acquisition Is Initialialization programming idiom". The main goal of this idiom is to ensure that resource acquisition occurs at the same time that the object is initialized, so that all resources for the object are created and made ready in one line of code. `std::unique_ptr` is a smart pointer that owns and manages another object through a pointer and disposes of that object when the unique_ptr goes out of scope. Ref 1: https://docs.microsoft.com/en-us/cpp/cpp/smart-pointers-modern-cpp Ref 2: http://en.cppreference.com/w/cpp/memory/unique_ptr
I think, we should use |
|
In `CMakeLists.txt`, include directory for example.h is added.
core/dronecore_impl.cpp
Outdated
@@ -330,7 +330,7 @@ void DroneCoreImpl::create_device_if_not_existing(const uint8_t system_id) | |||
} | |||
|
|||
// Create both lists in parallel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this comment makes no sense now :) I shall remove it in next commit.
This replaces raw pointers used in connections and devices by smart pointers. This avoids, freeing them in destructor and also makes code clean and readable.
Executed
|
Updates all the plugin API description about passing device object, instead of pointer to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, quite a lot of changes. Looks all correct though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Docs changes make sense. I think you caught the required "source based" changes, but I'll check more thoroughly after this is merged. Created mavlink/MAVSDK-docs#106 to track required docs changes.
@JonasVautherin when is a good time to merge this? 😄 |
I'm finishing my review now, I will merge it in a few minutes. Sorry for the delay =/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This commit replaces raw pointers by smart pointers and references as applicable.
We have to use references when we can, and pointers when we have to.
Smart pointers provide automatic resource management. In other words, they implement Resource Acquisition Is Initialization programming idiom. The main goal of this idiom is to ensure that resource acquisition occurs at the same time that the object is initialized, so that all resources for the object are created and made ready in one line of code.
std::unique_ptr
is a smart pointer that owns and manages another object through a pointer and disposes of that object when the unique_ptr goes out of scope.std::shared_ptr
is a smart pointer that retains shared ownership of an object through a pointer. Several shared_ptr objects may own the same object. The object is destroyed and its memory deallocated when either of the following happens:shared_ptr
owning the object is destroyed;shared_ptr
owning the object is assigned another pointer viaoperator=
orreset()
.References:
Fixes #314