From 541e657a041861c040693aadf9374051e4abcc12 Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Fri, 3 Nov 2017 21:14:28 +0000 Subject: [PATCH 1/8] Add has_been_met to the comparison operation - as an unmet goal and a met goal should never be equal --- src/Goal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Goal.py b/src/Goal.py index 54be8fd..b18ce45 100644 --- a/src/Goal.py +++ b/src/Goal.py @@ -34,4 +34,5 @@ def display(self, screen): def is_equal_to(self, other_goal): """Compare this Goal for equality with other_goal.""" return (self.sprite == other_goal.sprite and + self.has_been_met == other_goal.has_been_met and self.screen_location.is_equal_to(other_goal.screen_location)) From 2233b35432000eab49f133cb1a97ee8e782d354b Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Fri, 3 Nov 2017 21:16:22 +0000 Subject: [PATCH 2/8] Add unit tests for the Goal class --- test/test_goal.py | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 test/test_goal.py diff --git a/test/test_goal.py b/test/test_goal.py new file mode 100644 index 0000000..f864f18 --- /dev/null +++ b/test/test_goal.py @@ -0,0 +1,69 @@ +"""This file contains the TestGoal class.""" +import unittest +import pygame + +from src.Goal import Goal +from src.Point import Point + + +class TestGoal(unittest.TestCase): + """This test class unit tests the Goal class.""" + + @classmethod + def test_init(cls): + """Test the init method of the Goal class.""" + _unused_goal = Goal(None, Point(0, 0), 0) + + @classmethod + def test_display(cls): + """ + Test the display method of a Goal. + + All this really does is make sure the method executes correctly. + If the method call errors, the test will fail. + """ + # Create a test sprite + sprite = pygame.image.load('img/Default/goal1.jpg') + + # Create two test goals, one with a sprite and one without + goal_with_sprite = Goal(sprite, Point(1, 1), 150) + goal_without_sprite = Goal(None, Point(1, 1), 150) + + # Create a test screen to dsiplay things on + test_screen = pygame.display.set_mode((1500, 1500)) + + # Attempt to display the goal with a sprite + goal_with_sprite.display(test_screen) + + # Attempt to display the goal without a sprite + goal_without_sprite.display(test_screen) + + def test_is_equal_to(self): + """Test the is_equal_to_method.""" + # Create a test sprite + goal_sprite = pygame.image.load('img/Default/goal1.jpg') + + goal = Goal(goal_sprite, Point(1, 1), 150) + # Create a Goal equal to goal + goal_copy = Goal(goal_sprite, Point(1, 1), 150) + # Create a Goal in a different logical place + goal_diff_logical_position = Goal(goal_sprite, Point(2, 2), 150) + # Create a Goal that is met + met_goal = Goal(goal_sprite, Point(1, 1), 150) + met_goal.has_been_met = True + # Create a Goal with a different sprite + obstacle_sprite = pygame.image.load('img/Default/obstacle1.jpg') + goal_diff_sprite = Goal(obstacle_sprite, Point(1, 1), 150) + + # Check that equal goals are infact eqaul + self.assertTrue(goal.is_equal_to(goal_copy)) + # Check that a goal in a different logical place is not equal + self.assertFalse(goal.is_equal_to(goal_diff_logical_position)) + # Check an unmet Goal is not equal to a met Goal + self.assertFalse(goal.is_equal_to(met_goal)) + # Check Goals with different sprites are not equal + self.assertFalse(goal.is_equal_to(goal_diff_sprite)) + + +if __name__ == '__main__': + unittest.main() From f801f86559b0febe7f5054a52bf3806186712bc3 Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Sun, 3 Dec 2017 19:10:07 +0000 Subject: [PATCH 3/8] Stop is_equal_to assuming GoalGroups are ordered --- src/GoalGroup.py | 49 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/src/GoalGroup.py b/src/GoalGroup.py index 76cf8c8..5b00b20 100644 --- a/src/GoalGroup.py +++ b/src/GoalGroup.py @@ -55,7 +55,48 @@ def reset_all_goals(self): def is_equal_to(self, other_goal_group): """Compare this GoalGroup for equality with other_goal_group.""" - for i in range(0, len(self.goals)): - if not self.goals[i].is_equal_to(other_goal_group.goals[i]): - return False - return self.is_ordered == other_goal_group.is_ordered + if self.is_ordered and not other_goal_group.is_ordered: + # Then we can return False quickly as a ordered GoalGroup is + # different to an unordered GoalGroup. + return False + + if len(self.goals) is not len(other_goal_group.goals): + # Then we can return quickly as they are clearly not equal + return False + + if self.is_ordered: + # Then we can simply check that the elements are equal in order. + for i in range(0, len(self.goals)): + if not self.goals[i].is_equal_to(other_goal_group.goals[i]): + return False + + else: + # Then to be as general as possible, for each element in self.goals + # we check how many times it appears in both self.goals and + # other_goal_group.goals. If the counts are unequal, + # the GoalGroups are different. + for i in range(0, len(self.goals)): + elem = self.goals[i] + + # (Re)set variables to store the element counts + self_count = 0 + other_count = 0 + + # Look for elem in self.goals + for j in range(0, len(self.goals)): + if elem.is_equal_to(self.goals[j]): + # Each time elem is present, increment counter + self_count += 1 + + # Look for elem in other_goal_group.goals + for k in range(0, len(other_goal_group.goals)): + if elem.is_equal_to(other_goal_group.goals[k]): + # Each time elem is present, increment counter + other_count += 1 + + # If the counters aren't equal, the GoalGroups can't be equal. + if self_count != other_count: + return False + + # If we get here, then the GoalGroups are assumed to be equal. + return True From 8a60d360b576598dd6af5c0b9a30b6538a3b5c4a Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Sun, 3 Dec 2017 19:14:39 +0000 Subject: [PATCH 4/8] Add unittest for GoalGroup equality --- test/test_goal_group.py | 105 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 test/test_goal_group.py diff --git a/test/test_goal_group.py b/test/test_goal_group.py new file mode 100644 index 0000000..99b3d7e --- /dev/null +++ b/test/test_goal_group.py @@ -0,0 +1,105 @@ +"""This file contains the TestGoalGroup class.""" +import unittest +import pygame + +from src.Goal import Goal +from src.GoalGroup import GoalGroup +from src.Point import Point + + +class TestGoalGroup(unittest.TestCase): + """This test class unit tests the GoalGroup class.""" + + def setUp(self): + """Create GoalGroups and Goals used in testing.""" + # Create a test sprite + sprite = pygame.image.load('img/Default/goal1.jpg') + + # Create Goals + self.goal1 = Goal(sprite, Point(1, 1), 150) + self.goal2 = Goal(sprite, Point(2, 2), 150) + self.goal3 = Goal(sprite, Point(3, 2), 150) + + # Create a GoalGroup 'ordered' one way + self.goal_group1 = GoalGroup() + self.goal_group1.add(self.goal1) + self.goal_group1.add(self.goal2) + + # Create a GoalGroup 'ordered' the other way + self.goal_group2 = GoalGroup() + self.goal_group2.add(self.goal2) + self.goal_group2.add(self.goal1) + + def test_is_equal_to_eq_unord(self): + """Test two equal unordered GoalGroups for equality.""" + # Set the GoalGroups to unordered + self.goal_group1.is_ordered = False + self.goal_group2.is_ordered = False + + self.assertTrue(self.goal_group1.is_equal_to(self.goal_group2), + "Two equal unordered GoalGroups are not equal.") + + def test_is_equal_to_eq_ord(self): + """Test two equal ordered GoalGroups for equality.""" + # Set the GoalGroups to ordered + self.goal_group1.is_ordered = True + self.goal_group2.is_ordered = True + + self.assertFalse(self.goal_group1.is_equal_to(self.goal_group2), + "Two unequal ordered GoalGroups are equal.") + + def test_is_equal_to_mixed_order(self): + """Test two mixed ordered GoalGroups for equality.""" + # Set the GoalGroups so one is ordered and one is unordered + self.goal_group1.is_ordered = True + self.goal_group2.is_ordered = False + + self.assertFalse(self.goal_group1.is_equal_to(self.goal_group2), + "A ordered and unordered GoalGroup are equal.") + + def test_is_equal_to_uneq_unord(self): + """Test two unequal GoalGroups for equality.""" + # Create a unequal GoalGroup + unequal_goal_group = GoalGroup() + unequal_goal_group.add(self.goal2) + unequal_goal_group.add(self.goal3) + + # Set the GoalGroups to unordered. + self.goal_group1.is_ordered = False + unequal_goal_group.is_ordered = False + + self.assertFalse(self.goal_group1.is_equal_to(unequal_goal_group), + "Unequal GoalGroups are equal.") + + def test_is_equal_to_uneq_ord(self): + """Test two unequal GoalGroups for equality.""" + # Create a unequal GoalGroup + unequal_goal_group = GoalGroup() + unequal_goal_group.add(self.goal2) + unequal_goal_group.add(self.goal3) + + # Set the GoalGroups to ordered. + self.goal_group1.is_ordered = True + unequal_goal_group.is_ordered = True + + self.assertFalse(self.goal_group1.is_equal_to(unequal_goal_group), + "Unequal GoalGroups are equal.") + + def test_is_equal_to_uneq_larger(self): + """Test two differently sized GoalGroups for equality.""" + # Create a larger GoalGroup + larger_goal_group = GoalGroup() + larger_goal_group.add(self.goal1) + larger_goal_group.add(self.goal2) + larger_goal_group.add(self.goal3) + + # Set the GoalGroups to ordered + self.goal_group1.is_ordered = True + larger_goal_group.is_ordered = True + + self.assertFalse(self.goal_group1.is_equal_to(larger_goal_group), + "Different sized GoalGroups are equal.") + + +if __name__ == '__main__': + unittest.main() From 4bb4345fdc9390a2c4c52617fdb7d5e3f249a8bb Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Wed, 13 Dec 2017 20:17:27 +0000 Subject: [PATCH 5/8] Reduce Cognitive Complexity of is_equal_to method - split is_equal_to into two submethods - add a helper function to count the occurrences of a Goal in a GoalGroup --- src/GoalGroup.py | 93 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 31 deletions(-) diff --git a/src/GoalGroup.py b/src/GoalGroup.py index 5b00b20..6d73d95 100644 --- a/src/GoalGroup.py +++ b/src/GoalGroup.py @@ -65,38 +65,69 @@ def is_equal_to(self, other_goal_group): return False if self.is_ordered: - # Then we can simply check that the elements are equal in order. - for i in range(0, len(self.goals)): - if not self.goals[i].is_equal_to(other_goal_group.goals[i]): - return False + # Then return the ordered check + return self._is_equal_to_ordered(other_goal_group) - else: - # Then to be as general as possible, for each element in self.goals - # we check how many times it appears in both self.goals and - # other_goal_group.goals. If the counts are unequal, - # the GoalGroups are different. - for i in range(0, len(self.goals)): - elem = self.goals[i] - - # (Re)set variables to store the element counts - self_count = 0 - other_count = 0 - - # Look for elem in self.goals - for j in range(0, len(self.goals)): - if elem.is_equal_to(self.goals[j]): - # Each time elem is present, increment counter - self_count += 1 - - # Look for elem in other_goal_group.goals - for k in range(0, len(other_goal_group.goals)): - if elem.is_equal_to(other_goal_group.goals[k]): - # Each time elem is present, increment counter - other_count += 1 - - # If the counters aren't equal, the GoalGroups can't be equal. - if self_count != other_count: - return False + # If we get here, return the unordered check + return self._is_equal_to_unordered(other_goal_group) + + def _is_equal_to_ordered(self, ordered_goal_group): + """ + Compare the two GoalGroups for ordered equality. + + It does not check that either GoalGroup is ordered. + """ + # We can simply check that the elements are equal in order. + for i in range(0, len(self.goals)): + if not self.goals[i].is_equal_to(ordered_goal_group.goals[i]): + return False + + # If we get here, then the GoalGroups are assumed to be equal. + return True + + def _is_equal_to_unordered(self, unordered_goal_group): + """ + Compare the two GoalGroups for unordered equality. + + It does not check that either GoalGroup is unordered. + """ + # To be as general as possible, for each element in self.goals + # we check how many times it appears in both self.goals and + # other_goal_group.goals. If the counts are unequal, + # the GoalGroups are different. + for i in range(0, len(self.goals)): + elem = self.goals[i] + + # (Re)set variables to store the element counts + self_count = 0 + other_count = 0 + + # Look for elem in self.goals + self_count = self._count_occurrences(elem) + + # Look for elem in other_goal_group.goals + other_count = self._count_occurrences(elem, unordered_goal_group) + + # If the counters aren't equal, the GoalGroups can't be equal. + if self_count != other_count: + return False # If we get here, then the GoalGroups are assumed to be equal. return True + + def _count_occurrences(self, element, goal_group=None): + """ + Count the number of times the element appears in a GoalGroup. + + If no GoalGroup is provided, the method will check self. + """ + if goal_group is None: + goal_group = self + + count = 0 + for j in range(0, len(goal_group.goals)): + if element.is_equal_to(goal_group.goals[j]): + # Each time elem is present, increment counter + count += 1 + + return count From b127aae148e0a41f0e98659c64e8540f50006c23 Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Wed, 13 Dec 2017 23:38:32 +0000 Subject: [PATCH 6/8] Make the test sprite useable by all tests --- test/test_goal_group.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_goal_group.py b/test/test_goal_group.py index 99b3d7e..fe00bcb 100644 --- a/test/test_goal_group.py +++ b/test/test_goal_group.py @@ -13,12 +13,12 @@ class TestGoalGroup(unittest.TestCase): def setUp(self): """Create GoalGroups and Goals used in testing.""" # Create a test sprite - sprite = pygame.image.load('img/Default/goal1.jpg') + self.sprite = pygame.image.load('img/Default/goal1.jpg') # Create Goals - self.goal1 = Goal(sprite, Point(1, 1), 150) - self.goal2 = Goal(sprite, Point(2, 2), 150) - self.goal3 = Goal(sprite, Point(3, 2), 150) + self.goal1 = Goal(self.sprite, Point(1, 1), 150) + self.goal2 = Goal(self.sprite, Point(2, 2), 150) + self.goal3 = Goal(self.sprite, Point(3, 2), 150) # Create a GoalGroup 'ordered' one way self.goal_group1 = GoalGroup() From 959f8d82b948939b72fb672039ea2428b5d3080f Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Wed, 13 Dec 2017 23:29:52 +0000 Subject: [PATCH 7/8] Add a test for logically equal GoalGroups --- test/test_goal_group.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/test_goal_group.py b/test/test_goal_group.py index fe00bcb..1d06eb6 100644 --- a/test/test_goal_group.py +++ b/test/test_goal_group.py @@ -100,6 +100,30 @@ def test_is_equal_to_uneq_larger(self): self.assertFalse(self.goal_group1.is_equal_to(larger_goal_group), "Different sized GoalGroups are equal.") + def test_is_equal_to_same_goal(self): + """ + Test logically equal GoalGroups for equality. + + This particular test case catches when the underlying objects + are different but the GoalGroups are the same. + """ + # goalA is a different python object to self.goal1, + # but logically are the same. + same_goal = Goal(self.sprite, Point(1, 1), 150) + + # goal_groupA is a different python object to self.goal_group1, + # but logically are the same. + same_goal_group = GoalGroup() + same_goal_group.add(same_goal) + + # Set both GoalGroups to be un ordered + self.goal_group1.is_ordered = False + same_goal_group.is_ordered = False + + # GoalGroups that are logically the same should return True + # when compared via is_equal_to + self.assertTrue(self.goal_group1, same_goal_group) + if __name__ == '__main__': unittest.main() From 780b89f180296d8524ff54c522d474c66d847a2b Mon Sep 17 00:00:00 2001 From: gregcorbett Date: Tue, 5 Dec 2017 19:21:25 +0000 Subject: [PATCH 8/8] Update CHANGELOG --- CHANGELOG | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 7667c5e..24e1452 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,9 @@ +========== Next Version ========== + +Patches, Bug Fixes and Documentation Changes: + +- Fix bugs in Goal and GoalGroup is_equal_to methods + ========== Version 1.1.0 ========== New Features and Minor Changes: