BrewPi / firmware

Brewing temperature control firmware for the BrewPi Spark (Particle Photon inside)
http://www.brewpi.com
GNU Affero General Public License v3.0
97 stars 55 forks source link

Use smart pointers instead of raw pointers #45

Closed elcojacobs closed 7 years ago

elcojacobs commented 8 years ago

This PR is meant for discussion and should not be merged.

I have changed the actuator pointers from bare pointers to std::shared_ptr. The tests compile and succeed. The Core/Photon builds have not been fixed to compile .

While this was an interesting exercise, I doubt it is the right way forward.

Instead of sharing ownership between all objects interacting with an actuator (e.g. Control, PID and driver actuators), it would be better to use unique_ptr and have one true owner (the container). All other objects should reference the target actuator through this one true owner.

However, we might end up implementing a ref counting scheme similar to shared_ptr, to decide whether an object is not in use anymore and can be deleted.

If objects needing access to an actuator keep it as a reference member, do we only allow setting the reference during construction?

Articles worth reading: http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ http://herbsutter.com/2013/05/29/gotw-89-solution-smart-pointers/ http://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data

Erroneous1 commented 8 years ago

I recommend using std::unique_ptr wherever you can. You can make a vector of unique_ptrs which will delete when the vector goes out of scope. You could add to it by calling emplace or emplace_back, or use std::move. The cost of shared_ptr is pretty high comparatively speaking.

Also, check out Stroustrup's owner template. This lets you simply wrap your functions that expect pointers to designate when you return a new pointer or consume a pointer in a function and generate static assertions when you fail that. http://www.stroustrup.com/resource-model.pdf