Logical issue in createNode function of FlowScene

[arsdever]

Hi. IMO there is some conceptual issue in the FlowScene::createNode method. You see, in the method, a unique_ptr is created, and then a reference of the underlying object is returned.

But if you think about it, creating a unique_ptr means trying to be the only owner of the object. No other user should be able to hold the object.

In contrast, the createConnection method returns a shared_ptr. This also is a bit confusing. Some parts of the graph are returned uniquely, some not.

My suggestion would be to change the createNode implementation (maybe also interface) and create/return a shared_ptr. What do you think about this?

[paceholder]

Hi @arsdever ,

thanks for raising your concerns.

I think there is nothing wrong in returning a reference from this function. The semantic of the ownership stays valid -- FlowScene holds the created Node instance till the end.

The difference between Nodes and Connections is simple: Nodes are existing within the Scene and that's the end of the story. On the other hand, connections are stored in the Scene and at the same time, they are stored in the Nodes they are connected to.

The code exists in the current state for a somewhat long time and don't want to start changing the signatures of the core functions now.

In the version 3 (branch v3alpha) I've rewritten everything anyway. There the whole state is incapsulated in a single GraphModel and the user gets just the IDs of the created object (plain numbers 1, 2, 3, 4, ...).

Let me know what you think.

Best regards,

Dmitry