Skip to content

Use of NodeId for identifier#2

Open
XZETGEXI wants to merge 1 commit intohackl:masterfrom
XZETGEXI:patch-2
Open

Use of NodeId for identifier#2
XZETGEXI wants to merge 1 commit intohackl:masterfrom
XZETGEXI:patch-2

Conversation

@XZETGEXI
Copy link
Contributor

The method to identify nodes has two problems:

  • A node with a single-word name (like "Example") will have the same name and caption, since there are no empty spaces to replace with "_". This will trigger an "Identifier is not unique" in GeNIe.
  • Two nodes cannot have the same name, even if there are at different branches in the graph.

To solve these two problems, I suggest we use NodeId instead of node caption, for two reasons:

  • The internal mechanic of NodeId, incrementing each time a node is created, ensures that every ID is unique.
  • The name is displayed, not the ID, so the user interface is not changed too much.

Limitations:
Now the error will happen if you name a node something like "Node_1". But this is a more unlikely behaviour.

The method to identify nodes has two problems:
- A node with a single-word name (like "Example") will have the same name and caption, since there are no empty spaces to replace with "_". This will trigger an "Identifier is not unique" in GeNIe.
- Two nodes cannot have the same name, even if there are at different branches in the graph.

To solve these two problems, I suggest we use NodeId instead of node caption, for two reasons:
- The internal mechanic of NodeId, incrementing each time a node is created, ensures that every ID is unique.
- The name is displayed, not the ID, so the user interface is not changed too much.

Limitations:
Now the error will happen if you name a node something like "Node_1". But this is a more unlikely behaviour.
self.from_node = from_node
self.to_node = to_node
self.to_node.addArcConnection(self.from_node.getName(),self.from_node.getIdNum(),self.from_node.getSize())
self.to_node.addArcConnection(self.from_node.getName(),self.from_node.getNodeId(),self.from_node.getSize())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes what gets into the var of a node
This change might actually break how computeBeliefs work
Thus it is not necessary to make this change, even if redesigning computeBeliefs would be better in the long run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant