Do not delete a polymorphic object without a virtual destructor

[alf45tar]

May I suggest to add the following virtual destructor

virtual ~ButtonConfig() = default;

in order to avoid the following warning/error https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP52-CPP.+Do+not+delete+a+polymorphic+object+without+a+virtual+destructor ?

[bxparks]

The virtual destructor is required only if you create a ButtonConfig instance (or one of its subclasses) on the heap (using the new operator) and you are deleting it through the pointer. So can I ask, why are you deleting the ButtonConfig through a pointer? I designed this library so that objects are created as static instances, and are never destructed during the lifetime of the application.

The primary reason I explicitly do not include a virtual destructor because it causes the code size to bloat unnecessarily. It increases the flash memory size by about 600 bytes on 8-bit microcontrollers (ATmega328P and ATmega32U4). It also increases the static RAM size by 14 bytes. On the SAMD21, ESP8266 or ESP32, the flash memory increases only 28-60 bytes, which is what I would have expected due to the additional entry in the vtable.

Unfortunately, the 600 byte increase on the 8-bit processors is too much penalty for a functionality that I explicitly designed to not support in this library.

[alf45tar]

I am using an ESP32 board and I allocate from 0 to 36 buttons at run time with a normal value from 6 to 10. On 32 bit microcontroller it looks like it is better to allocate into the heap if the overhead is less than 60 bytes.

Not a big issue on ESP32 because we have enough memory and we can also statically allocate them.

Thanks
alf45tar

[bxparks]

I can see that allocating them on the heap can be convenient. But do you need to delete them at runtime? If you never delete them, you don't need a virtual destructor. Don't forget that even though the ESP32 has a lot more memory, you still need to worry about heap fragmentation. Especially if you use String objects which allocate longer strings on the heap.

If I still can't convince you that deleting from the heap is a bad idea, I'd willing to add a virtual destructor which is protected by a conditional macro, so that it is only activated for 32-bit processors.

[alf45tar]

I am deleting at runtime because my PedalinoMini project can be totally reconfigured without rebooting it.
I could reboot it because booting is very fast but a conditional macro for 32-bit processors should be the best.

[bxparks]

Ok, I can add the virtual destructor for ESP32 and other 32-bit processors.

I still think it's better to create the maximum number of AceButton and ButtonConfig objects at the start, then reconfigure them as needed, instead of deleting them from the heap. If a few AceButtons and ButtonConfigs are unused in certain configurations, no big deal, they have pretty small footprints. Probably cheaper than the heap fragmentation.

By the way, AceButton will not need a virtual destructor since it is not polymorphic. Only ButtonConfig needs a virtual destructor, and really, it's only to shut-up the compiler warnings, because there is currently no memory or resource leak even without a virtual destructor.