Skip to content

Graph weighted#11

Open
markreynoso wants to merge 22 commits into
masterfrom
graph_weighted
Open

Graph weighted#11
markreynoso wants to merge 22 commits into
masterfrom
graph_weighted

Conversation

@markreynoso

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread README.md
```


### Linkded List Class Usage

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo

Comment thread README.md
@@ -1,3 +1,6 @@
graph_weighted

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this up here?

Comment thread src/graph.py
"""Return a list of edges in graph."""
edges = {}
for key in self._graph:
for i in self._graph[key]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should be using a better variable name than "i" here

Comment thread src/graph.py
if val2 not in self._graph[val1]:
self._graph[val1][val2] = weight
elif val1 in self._graph and val2 not in self._graph:
self._graph[val2] = {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You have a method whose only purpose is to add nodes. Why not just call that method?

Comment thread src/graph.py
for key in self._graph:
for i in self._graph[key]:
if i == val:
del self._graph[key]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WHOA NOW! You're going to delete the whole node if the value you're looking for is found amongst its neighbors?!

Comment thread src/graph.py
else:
raise ValueError('These edges do not exist.')
except KeyError:
raise ValueError('These edges do not exist.')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're only deleting one edge

Comment thread src/graph.py

def has_node(self, val):
"""Return true or false if node has value."""
if val in self._graph:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The in operator returns a boolean value, so you could just

return val in self._graph

Comment thread src/graph.py
try:
for key in self._graph[val]:
neighbors.append(key)
return neighbors

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could just

return list(self._graph[val].keys())

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.

3 participants