Commit 2365e99d authored by Bram Schoenmakers's avatar Bram Schoenmakers

Look up parent todo items more efficiently

todo_by_dep_id was quite inefficient due to the linear search, calling
Todo.tag_value() many times to fetch the dependency ID.

Instead, maintain a dictionary that maps a parent ID to a todo item so
it can be looked up more easily.
parent 24a4227d
...@@ -330,6 +330,7 @@ class TodoListDependencyTester(TopydoTest): ...@@ -330,6 +330,7 @@ class TodoListDependencyTester(TopydoTest):
self.assertFalse(from_todo.has_tag('id')) self.assertFalse(from_todo.has_tag('id'))
self.assertFalse(to_todo.has_tag('p')) self.assertFalse(to_todo.has_tag('p'))
self.assertFalse(self.todolist.todo_by_dep_id('2'))
def test_remove_dep2(self): def test_remove_dep2(self):
old = str(self.todolist) old = str(self.todolist)
...@@ -338,6 +339,9 @@ class TodoListDependencyTester(TopydoTest): ...@@ -338,6 +339,9 @@ class TodoListDependencyTester(TopydoTest):
self.todolist.remove_dependency(from_todo, to_todo) self.todolist.remove_dependency(from_todo, to_todo)
self.assertEqual(str(self.todolist), old) self.assertEqual(str(self.todolist), old)
self.assertTrue(self.todolist.todo_by_dep_id('1'))
self.assertTrue(self.todolist.todo_by_dep_id('2'))
self.assertTrue(self.todolist.todo_by_dep_id('3'))
def test_remove_dep3(self): def test_remove_dep3(self):
""" Try to remove non-existing dependency. """ """ Try to remove non-existing dependency. """
...@@ -347,6 +351,9 @@ class TodoListDependencyTester(TopydoTest): ...@@ -347,6 +351,9 @@ class TodoListDependencyTester(TopydoTest):
self.todolist.remove_dependency(from_todo, to_todo) self.todolist.remove_dependency(from_todo, to_todo)
self.assertEqual(str(self.todolist), old) self.assertEqual(str(self.todolist), old)
self.assertTrue(self.todolist.todo_by_dep_id('1'))
self.assertTrue(self.todolist.todo_by_dep_id('2'))
self.assertTrue(self.todolist.todo_by_dep_id('3'))
def test_remove_todo_check_children(self): def test_remove_todo_check_children(self):
todo = self.todolist.todo(2) todo = self.todolist.todo(2)
...@@ -359,6 +366,7 @@ class TodoListDependencyTester(TopydoTest): ...@@ -359,6 +366,7 @@ class TodoListDependencyTester(TopydoTest):
todo = self.todolist.todo(3) todo = self.todolist.todo(3)
self.todolist.delete(todo) self.todolist.delete(todo)
self.assertFalse(todo.has_tag('p', '2')) self.assertFalse(todo.has_tag('p', '2'))
self.assertFalse(self.todolist.todo_by_dep_id('2'))
todo = self.todolist.todo(1) todo = self.todolist.todo(1)
children = self.todolist.children(todo) children = self.todolist.children(todo)
...@@ -427,19 +435,7 @@ class TodoListCleanDependencyTester(TopydoTest): ...@@ -427,19 +435,7 @@ class TodoListCleanDependencyTester(TopydoTest):
self.todolist.clean_dependencies() self.todolist.clean_dependencies()
self.assertFalse(self.todolist.todo(1).has_tag('id')) self.assertFalse(self.todolist.todo(1).has_tag('id'))
self.assertFalse(self.todolist.todo_by_dep_id('1'))
def test_clean_dependencies4(self):
""" Clean p: items when siblings are still connected to parent. """
self.todolist.add("Foo id:1")
self.todolist.add("Bar p:1")
self.todolist.add("Baz p:1 id:2")
self.todolist.add("Buzz p:2 p:1")
self.todolist.clean_dependencies()
self.assertFalse(self.todolist.todo(4).has_tag('p', '1'))
self.assertTrue(self.todolist.todo(1).has_tag('id', '1'))
self.assertTrue(self.todolist.todo(2).has_tag('p', '1'))
if __name__ == '__main__': if __name__ == '__main__':
......
...@@ -39,6 +39,7 @@ class TodoList(TodoListBase): ...@@ -39,6 +39,7 @@ class TodoList(TodoListBase):
""" """
# initialize these first because the constructor calls add_list # initialize these first because the constructor calls add_list
self._tododict = {} # hash(todo) to todo lookup self._tododict = {} # hash(todo) to todo lookup
self._parentdict = {} # dependency id => parent todo
self._depgraph = DirectedGraph() self._depgraph = DirectedGraph()
super().__init__(p_todostrings) super().__init__(p_todostrings)
...@@ -49,9 +50,10 @@ class TodoList(TodoListBase): ...@@ -49,9 +50,10 @@ class TodoList(TodoListBase):
There is only one such task, the behavior is undefined when a todo item There is only one such task, the behavior is undefined when a todo item
has more than one id tag. has more than one id tag.
""" """
hits = [t for t in self._todos if t.tag_value('id') == p_dep_id] try:
return self._parentdict[p_dep_id]
return hits[0] if len(hits) else None except KeyError:
return None
def _maintain_dep_graph(self, p_todo): def _maintain_dep_graph(self, p_todo):
""" """
...@@ -61,6 +63,7 @@ class TodoList(TodoListBase): ...@@ -61,6 +63,7 @@ class TodoList(TodoListBase):
dep_id = p_todo.tag_value('id') dep_id = p_todo.tag_value('id')
# maintain dependency graph # maintain dependency graph
if dep_id: if dep_id:
self._parentdict[dep_id] = p_todo
self._depgraph.add_node(hash(p_todo)) self._depgraph.add_node(hash(p_todo))
# connect all tasks we have in memory so far that refer to this # connect all tasks we have in memory so far that refer to this
...@@ -175,6 +178,7 @@ class TodoList(TodoListBase): ...@@ -175,6 +178,7 @@ class TodoList(TodoListBase):
if not self.children(p_from_todo, True): if not self.children(p_from_todo, True):
p_from_todo.remove_tag('id') p_from_todo.remove_tag('id')
del self._parentdict[dep_id]
self.dirty = True self.dirty = True
...@@ -220,6 +224,7 @@ class TodoList(TodoListBase): ...@@ -220,6 +224,7 @@ class TodoList(TodoListBase):
value = todo.tag_value('id') value = todo.tag_value('id')
if not self._depgraph.has_edge_id(value): if not self._depgraph.has_edge_id(value):
remove_tag(todo, 'id', value) remove_tag(todo, 'id', value)
del self._parentdict[value]
def clean_orphan_relations(): def clean_orphan_relations():
""" """
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment