diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 66da3f256..d37a17ffe 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -69,17 +69,14 @@ class CategoryActionList(object): """ Implement the __getitem__() method to make this class a dictionary type """ - try: - return self.actions[key][1] - except: - raise KeyError('Action "%s" does not exist.' % key) + return self.actions[key][1] - def __contains__(self, item): + def __contains__(self, key): """ Implement the __contains__() method to make this class a dictionary type """ for weight, action in self.actions: - if action == item: + if action == key: return True return False @@ -106,12 +103,6 @@ class CategoryActionList(object): self.index += 1 return self.actions[self.index - 1][1] - def has_key(self, key): - """ - Implement the has_key() method to make this class a dictionary type - """ - return key in self - def append(self, action): """ Append an action @@ -136,6 +127,7 @@ class CategoryActionList(object): if action[1] == remove_action: self.actions.remove(action) return + raise ValueError('Action "%s" does not exist.' % remove_action) class CategoryList(object): diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 8cc6df5da..890bde2ba 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -72,7 +72,7 @@ class TestCategoryActionList(TestCase): self.assertEqual(self.action1, returned_action) # THEN: Test if an exception is raised when trying to access a non existing item. - self.assertRaises(KeyError, self.list.__getitem__, 1) + self.assertRaises(IndexError, self.list.__getitem__, 1) def contains_test(self): """ @@ -141,12 +141,17 @@ class TestCategoryActionList(TestCase): Test the remove() method """ # GIVEN: The list + self.list.append(self.action1) + # WHEN: Delete an item from the list. self.list.remove(self.action1) # THEN: Now the element should not be in the list anymore. self.assertFalse(self.action1 in self.list) + # THEN: Check if an exception is raised when trying to remove a not present action. + self.assertRaises(ValueError, self.list.remove, self.action2) + class TestActionList(TestCase, TestMixin): """