search

paceholder / nodeeditor

Qt Node Editor. Dataflow programming framework
BSD 3-Clause "New" or "Revised" License
2.94k stars 796 forks source link

[suggestion/v3alpha] alternative to tuples #303

Closed facontidavide closed 2 years ago

facontidavide commented 2 years ago

Hi,

just an opinion. Using tuples make reading the code more verbose and less readable.

I would like to suggest this change:

struct ConnectionId
{
  NodeId out_nodeid;
  PortIndex out_index;
  NodeId in_nodeid;
  PortIndex in_index;
};

or maybe something like:

struct PortId
{
  NodeId nodeid;
  PortIndex index;
}
struct ConnectionId
{
  PortId out;
  PortId in;
};

From the point of view of the compiler, there is no difference, but the resulting code is more readable.

What do you think?

facontidavide commented 2 years ago

I can send a PR, if you think it is a valid idea

paceholder commented 2 years ago

If we replace std::make_tuple(a, b, c, d) with ConnectionId{a, b, c, d} it wouldn't make a big diffence.

On the other hand, when we read the properties, instead of get<2>(connectionId)we'd get connectionId.inNodeId which looks much better.

I agree with you, let's stick with

struct ConnectionId
{
  NodeId outNodeId;
  PortIndex outPortIndex;
  NodeId inNodeId;
  PortIndex inPortIndex;
};
facontidavide commented 2 years ago

I will submit a PR tomorrow