From dcb9de97e508aaa66b05004fcf25df750dff31d1 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 09:00:00 +0200 Subject: [PATCH 1/6] :adhesive_bandage: Pass params by vals instead of by refs if we pass parameters by reference, it must be because we want to change them in the method and to keep those changes. This seems like a different situation, so we should be able to pass the objects by value. --- Sources/Model/IManager.cs | 6 ++-- Sources/Model/PlayerManager.cs | 10 +++--- Sources/Tests/Model_UTs/PlayerManagerTest.cs | 32 ++++++++++---------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Sources/Model/IManager.cs b/Sources/Model/IManager.cs index ecc8ea9..2d7c13e 100644 --- a/Sources/Model/IManager.cs +++ b/Sources/Model/IManager.cs @@ -3,10 +3,10 @@ namespace Model { public interface IManager { - public T Add(ref T toAdd); + public T Add(T toAdd); public T GetOneById(int id); public IEnumerable GetAll(); - public T Update(ref T before, ref T after); - public void Remove(ref T toRemove); + public T Update(T before, T after); + public void Remove(T toRemove); } } diff --git a/Sources/Model/PlayerManager.cs b/Sources/Model/PlayerManager.cs index 93ab043..86eab00 100644 --- a/Sources/Model/PlayerManager.cs +++ b/Sources/Model/PlayerManager.cs @@ -26,7 +26,7 @@ namespace Model /// /// player to be added /// added player, or null if was null - public Player Add(ref Player toAdd) + public Player Add(Player toAdd) { if (toAdd != null) { @@ -76,17 +76,17 @@ namespace Model /// player to be updated /// player in the state that it needs to be in after the update /// updated player - public Player Update(ref Player before, ref Player after) + public Player Update(Player before, Player after) { - Remove(ref before); - return Add(ref after); + Remove(before); + return Add(after); } /// /// remove a player /// /// player to be removed - public void Remove(ref Player toRemove) + public void Remove(Player toRemove) { players.Remove(toRemove); } diff --git a/Sources/Tests/Model_UTs/PlayerManagerTest.cs b/Sources/Tests/Model_UTs/PlayerManagerTest.cs index b87d7d4..f34ff51 100644 --- a/Sources/Tests/Model_UTs/PlayerManagerTest.cs +++ b/Sources/Tests/Model_UTs/PlayerManagerTest.cs @@ -41,8 +41,8 @@ namespace Tests.Model_UTs // Act HashSet actual = new() { - playerManager.Add(ref alice), - playerManager.Add(ref bob) + playerManager.Add(alice), + playerManager.Add(bob) }; // Assert @@ -59,7 +59,7 @@ namespace Tests.Model_UTs // Act expected = null; - actual = playerManager.Add(ref expected); + actual = playerManager.Add(expected); // Assert Assert.Equal(expected, actual); @@ -89,7 +89,7 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player player = new("Bob"); - playerManager.Add(ref player); + playerManager.Add(player); // Act Player result = playerManager.GetOneByName(name); @@ -104,7 +104,7 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player player = new("Bob"); - playerManager.Add(ref player); + playerManager.Add(player); // Act Player result = playerManager.GetOneByName("Clyde"); @@ -123,7 +123,7 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player expected = new("Bob"); - playerManager.Add(ref expected); + playerManager.Add(expected); // Act Player actual = playerManager.GetOneByName(name); @@ -138,10 +138,10 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player p1 = new("Dylan"); - playerManager.Add(ref p1); + playerManager.Add(p1); // Act - playerManager.Remove(ref p1); + playerManager.Remove(p1); // Assert Assert.DoesNotContain(p1, playerManager.GetAll()); @@ -153,12 +153,12 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player player = new("Dylan"); - playerManager.Add(ref player); + playerManager.Add(player); Player notPlayer = null; HashSet expected = new() { player }; // Act - playerManager.Remove(ref notPlayer); + playerManager.Remove(notPlayer); HashSet actual = (HashSet)playerManager.GetAll(); // Assert @@ -171,12 +171,12 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player player = new("Dylan"); - playerManager.Add(ref player); + playerManager.Add(player); Player notPlayer = new("Eric"); HashSet expected = new() { player }; // Act - playerManager.Remove(ref notPlayer); + playerManager.Remove(notPlayer); HashSet actual = (HashSet)playerManager.GetAll(); // Assert @@ -189,11 +189,11 @@ namespace Tests.Model_UTs // Arrange PlayerManager playerManager = new(); Player oldPlayer = new("Dylan"); - playerManager.Add(ref oldPlayer); + playerManager.Add(oldPlayer); Player newPlayer = new("Eric"); // Act - playerManager.Update(ref oldPlayer, ref newPlayer); + playerManager.Update(oldPlayer, newPlayer); // Assert Assert.DoesNotContain(oldPlayer, playerManager.GetAll()); @@ -207,11 +207,11 @@ namespace Tests.Model_UTs string name = "Filibert"; PlayerManager playerManager = new(); Player oldPlayer = new(name); - playerManager.Add(ref oldPlayer); + playerManager.Add(oldPlayer); Player newPlayer = new(name); // Act - playerManager.Update(ref oldPlayer, ref newPlayer); + playerManager.Update(oldPlayer, newPlayer); // Assert Assert.Contains(oldPlayer, playerManager.GetAll()); -- 2.36.3 From f1404f829d0d7dfa82838506c9c8038d2623eef3 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 14:55:27 +0200 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=90=9B=20Add=20validation=20and=20del?= =?UTF-8?q?egation=20to=20Player.Remove()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Model/PlayerManager.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/Model/PlayerManager.cs b/Sources/Model/PlayerManager.cs index 86eab00..b14c07f 100644 --- a/Sources/Model/PlayerManager.cs +++ b/Sources/Model/PlayerManager.cs @@ -88,7 +88,12 @@ namespace Model /// player to be removed public void Remove(Player toRemove) { - players.Remove(toRemove); + // delegating, making sure we find it even if different case etc. + if (toRemove != null) + { + Player realToRemove = GetOneByName(toRemove.Name); + players.Remove(realToRemove); + } } } } -- 2.36.3 From 4472c5635422a29c758b66d6bb67525b44fbef51 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 14:57:37 +0200 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=85=20Improve=20Update()=20test=20aft?= =?UTF-8?q?er=20bugfix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Tests/Model_UTs/PlayerManagerTest.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Sources/Tests/Model_UTs/PlayerManagerTest.cs b/Sources/Tests/Model_UTs/PlayerManagerTest.cs index f34ff51..a8bb664 100644 --- a/Sources/Tests/Model_UTs/PlayerManagerTest.cs +++ b/Sources/Tests/Model_UTs/PlayerManagerTest.cs @@ -200,15 +200,19 @@ namespace Tests.Model_UTs Assert.Contains(newPlayer, playerManager.GetAll()); } - [Fact] - public void TestUpdateDoesNothingIfSame() + [Theory] + [InlineData("Filibert", "filibert")] + [InlineData("Filibert", " fiLibert")] + [InlineData("Filibert", "FIlibert ")] + [InlineData(" Filibert", " filiBErt ")] + public void TestUpdateDiscreetlyUpdatesCaseAndIgnoresExtraSpaceIfOtherwiseSame(string n1, string n2) { // Arrange string name = "Filibert"; PlayerManager playerManager = new(); - Player oldPlayer = new(name); + Player oldPlayer = new(n1); playerManager.Add(oldPlayer); - Player newPlayer = new(name); + Player newPlayer = new(n2); // Act playerManager.Update(oldPlayer, newPlayer); @@ -216,6 +220,8 @@ namespace Tests.Model_UTs // Assert Assert.Contains(oldPlayer, playerManager.GetAll()); Assert.Contains(newPlayer, playerManager.GetAll()); + Assert.Equal(n2.Trim(), playerManager.GetAll().First().Name); + // uses Equals(), which is made to be case-insensitive } } } -- 2.36.3 From 085e27122ac0b301885e8dbdb481328fd39c06b3 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 14:58:05 +0200 Subject: [PATCH 4/6] =?UTF-8?q?=E2=9C=8F=EF=B8=8F=20Fix=20typo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Tests/Model_UTs/PlayerManagerTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Tests/Model_UTs/PlayerManagerTest.cs b/Sources/Tests/Model_UTs/PlayerManagerTest.cs index a8bb664..0e788d0 100644 --- a/Sources/Tests/Model_UTs/PlayerManagerTest.cs +++ b/Sources/Tests/Model_UTs/PlayerManagerTest.cs @@ -208,7 +208,6 @@ namespace Tests.Model_UTs public void TestUpdateDiscreetlyUpdatesCaseAndIgnoresExtraSpaceIfOtherwiseSame(string n1, string n2) { // Arrange - string name = "Filibert"; PlayerManager playerManager = new(); Player oldPlayer = new(n1); playerManager.Add(oldPlayer); -- 2.36.3 From e3a86a67b9b6352012c4bf727aff65a66f9676c3 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 14:59:03 +0200 Subject: [PATCH 5/6] =?UTF-8?q?=E2=9C=85=20Generalize=20from=20HashSets=20?= =?UTF-8?q?to=20IEnumerables=20or=20Collections?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Tests/Model_UTs/PlayerManagerTest.cs | 33 +++++++++----------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/Sources/Tests/Model_UTs/PlayerManagerTest.cs b/Sources/Tests/Model_UTs/PlayerManagerTest.cs index 0e788d0..f3f87dc 100644 --- a/Sources/Tests/Model_UTs/PlayerManagerTest.cs +++ b/Sources/Tests/Model_UTs/PlayerManagerTest.cs @@ -1,29 +1,25 @@ using Model; using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; -using System.Text; -using System.Threading.Tasks; -using System.Xml.Linq; using Xunit; -using Xunit.Sdk; namespace Tests.Model_UTs { public class PlayerManagerTest { [Fact] - public void TestConstructorReturnsEmptyHashSet() + public void TestConstructorReturnsEmptyEnumerable() { // Arrange - PlayerManager playerManager; - HashSet expected; - HashSet actual; + PlayerManager playerManager = new(); + IEnumerable expected; + IEnumerable actual; // Act - playerManager = new(); - expected = new(); - actual = (HashSet)playerManager.GetAll(); + expected = new Collection(); + actual = playerManager.GetAll(); // Assert Assert.Equal(expected, actual); @@ -36,10 +32,10 @@ namespace Tests.Model_UTs PlayerManager playerManager = new(); Player alice = new("Alice"); Player bob = new("Bob"); - HashSet expected = new() { alice, bob }; // Act - HashSet actual = new() + Collection expected = new() { alice, bob }; + Collection actual = new() { playerManager.Add(alice), playerManager.Add(bob) @@ -59,7 +55,7 @@ namespace Tests.Model_UTs // Act expected = null; - actual = playerManager.Add(expected); + actual = playerManager.Add(expected);// Add() returns the added element if succesful // Assert Assert.Equal(expected, actual); @@ -155,11 +151,11 @@ namespace Tests.Model_UTs Player player = new("Dylan"); playerManager.Add(player); Player notPlayer = null; - HashSet expected = new() { player }; + IEnumerable expected = new Collection { player }; // Act playerManager.Remove(notPlayer); - HashSet actual = (HashSet)playerManager.GetAll(); + IEnumerable actual = playerManager.GetAll(); // Assert Assert.Equal(actual, expected); @@ -173,11 +169,11 @@ namespace Tests.Model_UTs Player player = new("Dylan"); playerManager.Add(player); Player notPlayer = new("Eric"); - HashSet expected = new() { player }; + IEnumerable expected = new Collection { player }; // Act playerManager.Remove(notPlayer); - HashSet actual = (HashSet)playerManager.GetAll(); + IEnumerable actual = playerManager.GetAll(); // Assert Assert.Equal(actual, expected); @@ -198,6 +194,7 @@ namespace Tests.Model_UTs // Assert Assert.DoesNotContain(oldPlayer, playerManager.GetAll()); Assert.Contains(newPlayer, playerManager.GetAll()); + Assert.True(playerManager.GetAll().Count() == 1); } [Theory] -- 2.36.3 From 10690dc9911173756bdc3bc673f6e46331817a70 Mon Sep 17 00:00:00 2001 From: Alexis DRAI Date: Mon, 19 Sep 2022 15:03:01 +0200 Subject: [PATCH 6/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor,=20D.R.Y.,=20?= =?UTF-8?q?use=20"expected"=20and=20"actual"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Tests/Model_UTs/PlayerTest.cs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Sources/Tests/Model_UTs/PlayerTest.cs b/Sources/Tests/Model_UTs/PlayerTest.cs index efe8011..9ab7bb8 100644 --- a/Sources/Tests/Model_UTs/PlayerTest.cs +++ b/Sources/Tests/Model_UTs/PlayerTest.cs @@ -11,12 +11,14 @@ namespace Tests.Model_UTs { // Arrange Player player; + string expected = "Alice"; // Act - player = new("Alice"); + player = new(expected); + string actual = player.Name; // Assert - Assert.Equal("Alice", player.Name); + Assert.Equal(expected, actual); } [Fact] @@ -24,12 +26,14 @@ namespace Tests.Model_UTs { // Arrange Player player; + string expected = "Alice"; // Act - player = new("Alice "); + player = new(" Alice "); + string actual = player.Name; // Assert - Assert.Equal("Alice", player.Name); + Assert.Equal(expected, actual); } [Theory] @@ -159,10 +163,11 @@ namespace Tests.Model_UTs // Arrange Player p1; Player p2; + string name = "Elyse"; // Act - p1 = new("Elyse"); - p2 = new("Elyse"); + p1 = new(name); + p2 = new(name); // Assert Assert.True(p1.Equals(p2)); @@ -211,10 +216,11 @@ namespace Tests.Model_UTs // Arrange Player p1; Player p2; + string name = "Elyse"; // Act - p1 = new("Elyse"); - p2 = new("Elyse"); + p1 = new(name); + p2 = new(name); // Assert Assert.True(p1.GetHashCode().Equals(p2.GetHashCode())); -- 2.36.3