scrime-u-bordeaux / libMidifilePerformer

C++ version of Midifile Performer core functionalities
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Make `MFPRenderer` inherit `Renderer` instead of using composition #13

Open josephlarralde opened 2 years ago

josephlarralde commented 2 years ago

This would be much more elegant and less error-prone if we didn't have to write T1 function(T2 arg) { return renderer.function(arg); } in MFPRenderer for each method of Renderer we want to expose.

After a bunch of failed attempts, it seems that it is not possible to achieve this using inheritance in C++.

I tried :

template <typename Model, typename Command, typename CommandKey>
class MFPRenderer : public Renderer<Model, Command, CommandKey> { /* ... */ }

and use a specialized version MFPRenderer<noteData, commandData, commandKey> with only explicit specializations of methods which required modifications.

I also tried :

class MFPRenderer : public Renderer<noteData, commandData, commandKey> { /* ... */ }

In both cases, we inherit from a class template we never fully explicitly specialize or instantiate, so all the functions that are not specialized with the concrete types are undefined, and although compilation succeeds with emscripten we end up with ... is not a function errors in the JS code.

I tried to create dummy instances of Renderer<noteData, commandData, commandKey> in the code (as static and local MFPRenderer variables), but this didn't help, maybe because of some compiler optimizations, or maybe because I don't grasp the issue very well ...

The only way to get inheritance to work (in both cases), is to write stuff like this for every public member

T1 function(T2 arg) { return Renderer<Model, Command, CommandKey>::function(arg); }

in the first case and

T1 function(T2 arg) { return Renderer<noteData, commandData, commandKey>::function(arg); }

in the second case which is as verbose and error-prone as the composition based design.

So, if anyone comes up with a better solution, I'm all ears. Meanwhile, I'll stick with the composition based design.

josephlarralde commented 2 years ago

PR #21 proposes an alternative lib architecture where we have simple Chronology and Renderer classes, and a new Performer class has a Chronology and a Renderer (or several of them, depending on the implementation). This is more flexible and reduces code duplication.