1
\$\begingroup\$

This is a recursive algorithm implementation of Eulerian tour search.

I guess there is no way to make it more efficient (except rewriting with loops instead of recursion). Any advice on style?

def sub(visited, _cur, graph):
 if not graph:
 return visited + [_cur]
 for i, edge in enumerate(graph):
 cur, nex = edge
 if _cur not in edge:
 continue
 _graph = graph[:]
 del _graph[i]
 if _cur == cur:
 res = sub(visited + [cur], nex, _graph)
 else:
 res = sub(visited + [nex], cur, _graph)
 if res:
 return res
def find_eulerian_tour(graph):
 head, tail = graph[0], graph[1:]
 prev, nex = head
 return sub([prev], nex, tail)
assert find_eulerian_tour([(1, 2), (2, 3), (3, 4), (4, 1)]) == [1, 2, 3, 4, 1]
assert find_eulerian_tour([
 (0, 1), (1, 5), (1, 7), (4, 5),
 (4, 8), (1, 6), (3, 7), (5, 9),
 (2, 4), (0, 4), (2, 5), (3, 6),
 (8, 9)
]) == [0, 1, 7, 3, 6, 1, 5, 4, 8, 9, 5, 2, 4, 0]
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 7, 2015 at 19:49
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Comments and good docstrings would go a long way. Obviously if I was familiar with the algorithm I'd understand easier, but you're not helping a lot with your naming. Why are you using _cur and cur? Do you just want to use the same name twice without clashing? If so you should do it another way, because _var is the Python style of indicating a variable that should be treated as private and not modified externally. It's an idiom for classes and looks out of place here.

Even still, cur, nex mean current and next, right? As you noted you shouldn't use next because it's a builtin name, but using next_branch or something more descriptive is better than stripping away a letter. Also if I've got the wrong end of the stick and nex is something else, that also just means you need a better shortened word or a comment explaining what nex is.

Also more whitespace will make the flow of your code easier to read than a solid block of text:

def sub(visited, _cur, graph):
 if not graph:
 return visited + [_cur]
 for i, edge in enumerate(graph):
 cur, nex = edge
 if _cur not in edge:
 continue
 _graph = graph[:]
 del _graph[i]
 if _cur == cur:
 res = sub(visited + [cur], nex, _graph)
 else:
 res = sub(visited + [nex], cur, _graph)
 if res:
 return res

Even before adding code and more readable names, this breaks up what's happening so a reader can parse it easier.

answered Sep 8, 2015 at 9:42
\$\endgroup\$
0
2
\$\begingroup\$
def find_euler_tour(visited,current,graph):
 queue=[current]
 while queue:
 if graph[current]:
 queue.append(current) 
 current=graph[current].pop()
 else:
 visited.append(current)
 current=queue.pop()
 return 

Thanks, changed your code to get rid of the recursive part, and I am using adjacency graphs (which have 2n edges) . Also you rebooted your graph matrix at every step, and I knew mine had 10000ish edges so, this way I am just slowly eating away at smaller lists inside of a dictionary of children vertices connected to parent vertices (adjacency graph).

Graph was created like this: assumming a 2-tuple for each edge between node A,B

from collections import defaultdict
graph=defaultdict(list)
for A,B in edges:
 graph[A].append(B)
 graph[B].append(A)

Called like

visited=[]
current=1 #starting at Node 1 for example
find_euler_tour(visited,current,graph)

I was after a complete n-ary tree eulerian walk through a undirected tree graph. First step toward Least Common Ancestor.

answered Mar 3, 2017 at 21:13
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.