# Is there a redundancy between the two functions of adding a start node and an end node to my graph?

I feel like there is a redundency between:

def _determine_nodes_with_no_predecessors(nodes:List[Node])->List[str]:
"""
Given the list of nodes returns the identifiers of nodes that doesn't have any predecessor.
"""
nodes_ids_without_prdecessors = []
for node in nodes:
if not node.predecessors:
nodes_ids_without_prdecessors.append(node.id_)
return nodes_ids_without_prdecessors

start_node = Node.create_start_node(graph)

nodes_ids_without_prdecessors = _determine_nodes_with_no_predecessors(graph.nodes)

# Its successors are the nodes without predecessors in this graph
for node_id in nodes_ids_without_prdecessors:

# Conversly, We add the start_node as a predecessor to the nodes without predecessors in this graph
for node_id in nodes_ids_without_prdecessors:
node = graph.nodes[node_id]

And this one:

def _determine_nodes_with_no_successors(nodes:List[Node])->List[str]:
"""
Given the list of nodes returns the identifiers of nodes that doesn't have any successor.
"""
nodes_ids_without_successors = []
for node in nodes:
if not node.successors:
nodes_ids_without_successors.append(node.id_)
return nodes_ids_without_successors

end_node = Node.create_end_node() #constructor of node

end_node = graph.nodes[end_node.id_]

nodes_ids_without_successors = _determine_nodes_with_no_successors(graph.nodes)

# Its predecessors are the nodes without successors in this graph
for node_id in nodes_ids_without_successors:

# Conversly, We add the end_node as a predecessor to the nodes without predecessors in this graph
for node_id in nodes_ids_without_successors:
node = graph.nodes[node_id]

Note: Node is a class that have the attributes predecessors, successors and id_ and the class methods create_dummy_start_node, create_dummy_end_node, add_predecessor and add_successor. Graph is a class that have the attribute nodes and the method add node.

How to write less code in this case? Is it possible to write only two functions that do the whole stuff (instead of four functions)?

You can use the built-in getattr function to extract an attribute by name. Likewise the setattr function to set attributes.

This means you can pass the name of the attribute to a private function, unifying the code:

def _find_nodes_with_no(pred_succ: str, nodes: List[Node]) -> List[Node]:
""" Find nodes with either no predecessors, or no successors. """

if pred_succ not in ("predecessors", "successors"):
raise ValueError("pred_succ must be either 'predecessors' or 'successors'")

return [node for node in nodes if not getattr(node, pred_succ)]

info = ("create_start_node", "predecessors", "successors")

info = ("create_end_node", "successors", "predecessors")

Then in _add_outer_node you can do things like:

create_method = getattr(Node, cmeth)
new_node = create_method(graph)

# ...

One thing I do question it your "manual" connection of both the predecessor and successor links. It seems like you want those to be reflexive, so why not create an add_predecessor method that automatically creates the reciprocal link? Likewise with add_successor?

Since there's only two cases, the strategy pattern should be sufficient. Alternatively, creating an inverted view of the graph would allow you to implement _add_end_node() by calling _add_start_node() on the inverted graph view.

class StartNodeStrategy:
def create_node(self):
return Node.create_start_node()
def find_hanging(self,nodes):
return [
node.id_
for node in nodes
if not node.predecessor
]
def fix_hanging(self,outer_node,inner_node):

class EndNodeStrategy:
def create_node(self):
return Node.create_end_node()
def find_hanging(self,nodes):
return [
node.id_
for node in nodes
if not node.successor
]
def fix_hanging(self,outer_node,inner_node):