Michael Marcin wrote:
I have a new programmer on my team who has been brought on as an optimization engineer. She has a lot more experience than I do but I fear some of that experience may have left impressions that have stuck with her for too long. On the first check in labeled spot optimizations there were many things changed that disturbed me. One of them was changing a class from essentially.
#include
class Context { public: Context( unsigned int width, unsigned int height ) : m_buffer( new unsigned int[width*height] ) { } unsigned int* GetBuffer() { return m_buffer.get(); } private: boost::scoped_array<unsigned int> m_buffer; };
to
class Context { public: Context( unsigned int width, unsigned int height ) : m_buffer( new unsigned int[width*height] ) { } ~Context() { delete m_buffer; }
inline unsigned int* GetBuffer() { return m_buffer; } private: unsigned int* m_buffer; };
First of all there is the defect of delete being called instead of delete[]. Other than that I was under the impression that there should be no difference between the two other than the former being more descriptive (it instantly tells me this is an array and the memory is owned by the instance of this class).
Is this really an optimization?
Aside from the mistaken form of delete, your engineer is missing the whole point of scoped_array<>. For instance, what happens when a program copies a Context (which can happen easily in various expressions). The copy constructor will copy the pointer, and then you will have two Context objects that will both delete the same array. Someone had a good reason for using scoped_array. Don't throw it away due to someone's not understanding the reason(s). -- Dick Hadsell 914-259-6320 Fax: 914-259-6499 Reply-to: hadsell@blueskystudios.com Blue Sky Studios http://www.blueskystudios.com 44 South Broadway, White Plains, NY 10601