search

openAVproductions / openAV-Luppp

Luppp is a live performance tool, created by OpenAV productions.
http://openavproductions.com/luppp
GNU General Public License v3.0
258 stars 45 forks source link

memory allocation in header files #157

Closed georgkrause closed 7 years ago

georgkrause commented 7 years ago

@harryhaaren told me allocating in header files is wrong. so i started to rework the code where i did this. so i searched for the string "= new" in all .hxx files and found one place which is in the master branch right now.

https://github.com/openAVproductions/openAV-Luppp/blob/master/src/gunittrack.hxx#L47

i dont know if this is the same mistake i did or not, i simple wanted to write it down so someone can look at this ;)

harryhaaren commented 7 years ago

Thanks for your digging in the source - it always leads to interesting things :)

The code in question here implements the GUnitTrack() function. You are correct it implements it in the header file.The comment made (https://github.com/openAVproductions/openAV-Luppp/pull/145#pullrequestreview-11294844) about using new in a header file is bad when you are only declaring a variable.

The difference is subtle, but it does make a difference. In the GUnitTrack case, the constructor function allocates memory - that's OK. In the LooperClip case, memory is allocated outside a function. This is not clean programming - even if the compiler makes assumptions on what you actually intended and still does the right thing xD

I don't want to "defend" my code here - I'm sure there's lots of places where things can be improved, and my mistakes corrected - in this case there is a subtle difference that is a very good interview question... I'll learn from this, and ask it if I ever get to interview people :smiley:

Thanks again for your looking around the codebase! -H

georgkrause commented 7 years ago

thanks for the explanation!