Suggestion: use Curiously Recurring Template Pattern (CRTP)

[facontidavide]

Hi,

I just wanted to share with you my two cents. I have a code very similar to yours in terms or Registry (what other proplr call Factory).

NodeDataModel has pure virtual methods that derived classes need to implement. These method are const and looking at them, it is reasonable to expect them to be static methods, since they would hardly need to deal with this IMHO

In particular name() , nPorts and dataType seems to be attributes of the class itself and not the instance.

Using CRTP you can make them static. Unfortunately this would be a breaking change in the API.

What do you think?

[russelltg]

In my code, I (used to, before I formed #117) did have those vary per instance because I had data types that were created at runtime, so had to use one common class for each data type.

I'm not sure if that's a common enough use case, but it does exist.

-Russell

On Thu, Dec 7, 2017, 6:55 AM Davide Faconti [email protected] wrote:

Hi,

I just wanted to share with you my two cents. I have a code very similar to yours in terms or Registry (what other proplr call Factory).

NodeDataModel has pure virtual methods that derived classes need to implement. These method are const and looking at them, it is reasonable to expect them to be static methods, since they would hardly need to deal with this IMHO

In particular nPorts and dataType seems to be attributes of the class itself and not the instance.

Using CRTP you can make them static. Unfortunately this would be a breaking change in the API.

What do you think?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/paceholder/nodeeditor/issues/136, or mute the thread https://github.com/notifications/unsubscribe-auth/AGxqOkw_lN5fREHW1vRHzLyPcw8W3JXWks5s9-5hgaJpZM4Q5lYR .

[facontidavide]

ok, in that case it is better not to brake people's code. Never mind