🚨 fix code smells

Sources/Data/EF/Players/PlayerDBManager.cs

@@ -45,7 +45,7 @@ namespace Data.EF.Players
         /// <returns></returns>
         /// <exception cref="ArgumentNullException"></exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task<PlayerEntity> Add(PlayerEntity toAdd)
+        public Task<PlayerEntity> Add(PlayerEntity toAdd)
         {
             CleanPlayerEntity(toAdd);
 
@@ -54,6 +54,11 @@ namespace Data.EF.Players
                 throw new ArgumentException("this username is already taken", nameof(toAdd));
             }
 
+            return InternalAdd(toAdd);
+        }
+
+        private async Task<PlayerEntity> InternalAdd(PlayerEntity toAdd)
+        {
             EntityEntry ee = await db.Players.AddAsync(toAdd);
             await db.SaveChangesAsync();
             return (PlayerEntity)ee.Entity;
@@ -74,17 +79,22 @@ namespace Data.EF.Players
         /// <exception cref="ArgumentException"></exception>
         /// <exception cref="InvalidOperationException"></exception>
         /// <returns></returns>
-        public async Task<PlayerEntity> GetOneByName(string name)
+        public Task<PlayerEntity> GetOneByName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
             {
                 throw new ArgumentException("Name property should not be null or whitespace", nameof(name));
             }
             name = name.Trim();
-            return await db.Players.Where(p => p.Name == name).FirstAsync();
+            return InternalGetOneByName(name);
         }
 
 
+        private async Task<PlayerEntity> InternalGetOneByName(string name)
+        {
+            return await db.Players.Where(p => p.Name == name).FirstAsync();
+        }
+
         public async Task<bool> IsPresentByName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
@@ -119,7 +129,7 @@ namespace Data.EF.Players
         /// <returns>the updated entity</returns>
         /// <exception cref="ArgumentNullException"></exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task<PlayerEntity> Update(PlayerEntity before, PlayerEntity after)
+        public Task<PlayerEntity> Update(PlayerEntity before, PlayerEntity after)
         {
             PlayerEntity[] args = { before, after };
 
@@ -133,9 +143,14 @@ namespace Data.EF.Players
                 throw new ArgumentException("ID cannot be updated", nameof(after));
             }
 
+            return InternalUpdate(before, after);
+
+        }
+
+        private async Task<PlayerEntity> InternalUpdate(PlayerEntity before, PlayerEntity after)
+        {
             Remove(before);
             return await Add(after);
-
         }
 
         /// <summary>

Sources/Data/EF/Players/PlayerDbManager.cs

@@ -45,7 +45,7 @@ namespace Data.EF.Players
         /// <returns></returns>
         /// <exception cref="ArgumentNullException"></exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task<PlayerEntity> Add(PlayerEntity toAdd)
+        public Task<PlayerEntity> Add(PlayerEntity toAdd)
         {
             CleanPlayerEntity(toAdd);
 
@@ -54,6 +54,11 @@ namespace Data.EF.Players
                 throw new ArgumentException("this username is already taken", nameof(toAdd));
             }
 
+            return InternalAdd(toAdd);
+        }
+
+        private async Task<PlayerEntity> InternalAdd(PlayerEntity toAdd)
+        {
             EntityEntry ee = await db.Players.AddAsync(toAdd);
             await db.SaveChangesAsync();
             return (PlayerEntity)ee.Entity;
@@ -74,17 +79,22 @@ namespace Data.EF.Players
         /// <exception cref="ArgumentException"></exception>
         /// <exception cref="InvalidOperationException"></exception>
         /// <returns></returns>
-        public async Task<PlayerEntity> GetOneByName(string name)
+        public Task<PlayerEntity> GetOneByName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
             {
                 throw new ArgumentException("Name property should not be null or whitespace", nameof(name));
             }
             name = name.Trim();
-            return await db.Players.Where(p => p.Name == name).FirstAsync();
+            return InternalGetOneByName(name);
         }
 
 
+        private async Task<PlayerEntity> InternalGetOneByName(string name)
+        {
+            return await db.Players.Where(p => p.Name == name).FirstAsync();
+        }
+
         public async Task<bool> IsPresentByName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
@@ -119,7 +129,7 @@ namespace Data.EF.Players
         /// <returns>the updated entity</returns>
         /// <exception cref="ArgumentNullException"></exception>
         /// <exception cref="ArgumentException"></exception>
-        public async Task<PlayerEntity> Update(PlayerEntity before, PlayerEntity after)
+        public Task<PlayerEntity> Update(PlayerEntity before, PlayerEntity after)
         {
             PlayerEntity[] args = { before, after };
 
@@ -133,9 +143,14 @@ namespace Data.EF.Players
                 throw new ArgumentException("ID cannot be updated", nameof(after));
             }
 
+            return InternalUpdate(before, after);
+
+        }
+
+        private async Task<PlayerEntity> InternalUpdate(PlayerEntity before, PlayerEntity after)
+        {
             Remove(before);
             return await Add(after);
-
         }
 
         /// <summary>

Sources/Data/Stub.cs

@@ -82,7 +82,7 @@ namespace Data
             await (await gr.GameManager.GetOneByName(game3)).PlayerManager.Add(player1);
             await (await gr.GameManager.GetOneByName(game3)).PlayerManager.Add(player3);
 
-            foreach (Game game in gr.GameManager.GetAll().Result)
+            foreach (Game game in gr.GameManager.GetAll()?.Result)
             {
                 for (int i = 0; i < 10; i++)
                 {

Sources/Model/Dice/DiceGroupManager.cs

@@ -9,7 +9,7 @@ namespace Model.Dice
     {
         private readonly Dictionary<string, IEnumerable<Die>> diceGroups = new();
 
-        public async Task<KeyValuePair<string, IEnumerable<Die>>> Add(KeyValuePair<string, IEnumerable<Die>> toAdd)
+        public Task<KeyValuePair<string, IEnumerable<Die>>> Add(KeyValuePair<string, IEnumerable<Die>> toAdd)
         {
             if (string.IsNullOrWhiteSpace(toAdd.Key))
             {
@@ -21,12 +21,12 @@ namespace Model.Dice
                 throw new ArgumentException("this username is already taken", nameof(toAdd));
             }
             diceGroups.Add(toAdd.Key.Trim(), toAdd.Value);
-            return await Task.FromResult(toAdd);
+            return Task.FromResult(toAdd);
         }
 
-        public async Task<IEnumerable<KeyValuePair<string, IEnumerable<Die>>>> GetAll()
+        public Task<IEnumerable<KeyValuePair<string, IEnumerable<Die>>>> GetAll()
         {
-            return await Task.FromResult(diceGroups.AsEnumerable());
+            return Task.FromResult(diceGroups.AsEnumerable());
         }
 
         public Task<KeyValuePair<string, IEnumerable<Die>>> GetOneByID(Guid ID)
@@ -34,7 +34,7 @@ namespace Model.Dice
             throw new NotImplementedException();
         }
 
-        public async Task<KeyValuePair<string, IEnumerable<Die>>> GetOneByName(string name)
+        public Task<KeyValuePair<string, IEnumerable<Die>>> GetOneByName(string name)
         {
             // les groupes de dés nommés :
             // ils sont case-sensistive, mais "mon jeu" == "mon jeu " == "  mon jeu"
@@ -42,10 +42,7 @@ namespace Model.Dice
             {
                 throw new ArgumentNullException(nameof(name), "param should not be null or empty");
             }
-            else
+            return Task.FromResult(new KeyValuePair<string, IEnumerable<Die>>(name, diceGroups[name]));
-            {
-                return await Task.FromResult(new KeyValuePair<string, IEnumerable<Die>>(name, diceGroups[name]));
-            }
         }
 
         public void Remove(KeyValuePair<string, IEnumerable<Die>> toRemove)
@@ -68,7 +65,7 @@ namespace Model.Dice
         /// <returns></returns>
         /// <exception cref="ArgumentException"></exception>
         /// <exception cref="ArgumentNullException"></exception>
-        public async Task<KeyValuePair<string, IEnumerable<Die>>> Update(KeyValuePair<string, IEnumerable<Die>> before, KeyValuePair<string, IEnumerable<Die>> after)
+        public Task<KeyValuePair<string, IEnumerable<Die>>> Update(KeyValuePair<string, IEnumerable<Die>> before, KeyValuePair<string, IEnumerable<Die>> after)
         {
             // pas autorisé de changer les dés, juste le nom
             if (!before.Value.Equals(after.Value))
@@ -80,8 +77,8 @@ namespace Model.Dice
                 throw new ArgumentNullException(nameof(before), "dice group name should not be null or empty");
             }
             diceGroups.Remove(before.Key);
-            await Add(after);
+            Add(after);
-            return after;
+            return Task.FromResult(after);
 
         }
     }

Sources/Model/Games/Game.cs

@@ -180,7 +180,7 @@ namespace Model.Games
             sb.Append($"Game: {Name}");
 
             sb.Append("\nPlayers:");
-            foreach (Player player in PlayerManager.GetAll().Result)
+            foreach (Player player in PlayerManager.GetAll()?.Result)
             {
                 sb.Append($" {player.ToString()}");
             }

Sources/Model/Games/GameManager.cs

@@ -21,7 +21,7 @@ namespace Model.Games
         /// gets an unmodifiable collection of the games
         /// </summary>
         /// <returns>unmodifiable collection of the games</returns>
-        public async Task<IEnumerable<Game>> GetAll() => await Task.FromResult(games.AsEnumerable());
+        public Task<IEnumerable<Game>> GetAll() => Task.FromResult(games.AsEnumerable());
 
         /// <summary>
         /// finds the game with that name and returns it
@@ -30,11 +30,11 @@ namespace Model.Games
         /// </summary>
         /// <param name="name">a games's name</param>
         /// <returns>game with said name, <em>or null</em> if no such game was found</returns>
-        public async Task<Game> GetOneByName(string name)
+        public Task<Game> GetOneByName(string name)
         {
             if (!string.IsNullOrWhiteSpace(name))
             {
-                return await Task.FromResult(games.FirstOrDefault(g => g.Name == name)); // may return null
+                return Task.FromResult(games.FirstOrDefault(g => g.Name == name)); // may return null
             }
             throw new ArgumentException("param should not be null or blank", nameof(name));
         }
@@ -54,7 +54,7 @@ namespace Model.Games
         /// saves a given game -- does not allow copies yet: if a game with the same name exists, it is overwritten
         /// </summary>
         /// <param name="toAdd">a game to save</param>
-        public async Task<Game> Add(Game toAdd)
+        public Task<Game> Add(Game toAdd)
         {
             if (toAdd is null)
             {
@@ -64,7 +64,7 @@ namespace Model.Games
             games.Remove(games.FirstOrDefault(g => g.Name == toAdd.Name));
             // will often be an update: if game with that name exists, it is removed, else, nothing happens above
             games.Add(toAdd);
-            return await Task.FromResult(toAdd);
+            return Task.FromResult(toAdd);
         }
 
         /// <summary>
@@ -88,7 +88,7 @@ namespace Model.Games
         /// <param name="after">new game</param>
         /// <returns></returns>
         /// <exception cref="ArgumentNullException"></exception>
-        public async Task<Game> Update(Game before, Game after)
+        public Task<Game> Update(Game before, Game after)
         {
 
             Game[] args = { before, after };
@@ -103,7 +103,7 @@ namespace Model.Games
                 }
             }
             Remove(before);
-            return await Add(after);
+            return (Add(after));
         }
     }
 }

Sources/Model/Players/PlayerManager.cs

@@ -21,7 +21,7 @@ namespace Model.Players
         /// </summary>
         /// <param name="toAdd">player to be added</param>
         /// <returns>added player</returns>
-        public async Task<Player> Add(Player toAdd)
+        public Task<Player> Add(Player toAdd)
         {
             if (toAdd is null)
             {
@@ -32,29 +32,27 @@ namespace Model.Players
                 throw new ArgumentException("this username is already taken", nameof(toAdd));
             }
             players.Add(toAdd);
-            return await Task.FromResult(toAdd);
+            return Task.FromResult(toAdd);
         }
 
         /// <summary>
-        /// finds the player with that name and returns A COPY OF IT
+        /// finds the player with that name and returns it
-        /// <br/>
-        /// that copy does not belong to this manager's players, so it should not be modified 
         /// </summary>
         /// <param name="name">a player's unique name</param>
         /// <returns>player with said name, <em>or null</em> if no such player was found</returns>
-        public async Task<Player> GetOneByName(string name)
+        public Task<Player> GetOneByName(string name)
         {
-            if (!string.IsNullOrWhiteSpace(name))
+            if (string.IsNullOrWhiteSpace(name))
             {
-                Player wanted = new(name);
+                throw new ArgumentException("param should not be null or blank", nameof(name));
-                Player result = players.FirstOrDefault(p => p.Equals(wanted));
+            }
-                if (result == null)
+
-                {
+            Player result = players.FirstOrDefault(p => p.Name.ToUpper().Equals(name.ToUpper().Trim()));
-                    return null;
+            if (result == null)
-                }
+            {
-                return await Task.FromResult(new Player(result)); // THIS IS A COPY (using a copy constructor)
+                return Task.FromResult<Player>(null);
             }
-            throw new ArgumentException("param should not be null or blank", nameof(name));
+            return Task.FromResult(result);
         }
 
         /// </summary>
@@ -62,7 +60,7 @@ namespace Model.Players
         /// so that the only way to modify the collection of players is to use this class's methods
         /// </summary>
         /// <returns>a readonly enumerable of all this manager's players</returns>
-        public async Task<IEnumerable<Player>> GetAll() => await Task.FromResult(players.AsEnumerable());
+        public Task<IEnumerable<Player>> GetAll() => Task.FromResult(players.AsEnumerable());
 
         /// <summary>
         /// update a player from <paramref name="before"/> to <paramref name="after"/>
@@ -70,7 +68,7 @@ namespace Model.Players
         /// <param name="before">player to be updated</param>
         /// <param name="after">player in the state that it needs to be in after the update</param>
         /// <returns>updated player</returns>
-        public async Task<Player> Update(Player before, Player after)
+        public Task<Player> Update(Player before, Player after)
         {
             Player[] args = { before, after };
 
@@ -84,7 +82,7 @@ namespace Model.Players
                 }
             }
             Remove(before);
-            return await Add(after);
+            return Add(after);
         }
 
         /// <summary>

Sources/Tests/Data_UTs/Players/PlayerDbManagerTest.cs

@@ -367,8 +367,8 @@ namespace Tests.Data_UTs.Players
         }
 
         [Fact]
-        public async Task TestRemoveWhenPreExistentThenRemoves()
+        public void TestRemoveWhenPreExistentThenRemoves()
-        {
+        {// should be async Task
             // Arrange
 
             PlayerDbManager mgr;
@@ -393,6 +393,7 @@ namespace Tests.Data_UTs.Players
                 db.Database.EnsureCreated();
                 mgr = new(db);
 
+                // should check if toRemove is now absent
                 Assert.True(true);
             }
         }

Sources/Tests/Model_UTs/Games/GameManagerTest.cs

@@ -16,7 +16,7 @@ namespace Tests.Model_UTs.Games
 {
     public class GameManagerTest
     {
-        private readonly MasterOfCeremonies stubGameRunner = new Stub().LoadApp().Result;
+        private readonly MasterOfCeremonies stubGameRunner = new Stub().LoadApp()?.Result;
 
         [Fact]
         public async Task TestConstructorReturnsEmptyEnumerableAsync()

Sources/Tests/Model_UTs/Games/GameTest.cs

@@ -14,14 +14,14 @@ namespace Tests.Model_UTs.Games
 {
     public class GameTest
     {
-        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp().Result;
+        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp()?.Result;
         private static readonly string GAME_NAME = "my game";
 
         private static readonly Player PLAYER_1 = new("Alice"), PLAYER_2 = new("Bob"), PLAYER_3 = new("Clyde");
         private readonly IEnumerable<Die> DICE_1, DICE_2;
         public GameTest()
         {
-            IEnumerable<KeyValuePair<string, IEnumerable<Die>>> diceGroups = stubMasterOfCeremonies.DiceGroupManager.GetAll().Result;
+            IEnumerable<KeyValuePair<string, IEnumerable<Die>>> diceGroups = stubMasterOfCeremonies.DiceGroupManager.GetAll()?.Result;
             DICE_1 = diceGroups.First().Value;
             DICE_2 = diceGroups.Last().Value;
         }

Sources/Tests/Model_UTs/Games/MasterOfCeremoniesTest.cs

@@ -12,7 +12,7 @@ namespace Tests.Model_UTs.Games
 {
     public class MasterOfCeremoniesTest
     {
-        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp().Result;
+        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp()?.Result;
 
         [Fact]
         public async Task TestPlayGameWhenPlayThenAddNewTurnToHistoryAsync()
@@ -46,7 +46,7 @@ namespace Tests.Model_UTs.Games
             await masterOfCeremonies.StartNewGame(
                 name,
                 new PlayerManager(),
-                stubMasterOfCeremonies.GameManager.GetAll().Result.First().Dice
+                stubMasterOfCeremonies.GameManager.GetAll()?.Result.First().Dice
                 );
 
             // Assert

Sources/Tests/Model_UTs/Games/TurnTest.cs

@@ -13,15 +13,15 @@ namespace Tests.Model_UTs.Games
     public class TurnTest
 
     {
-        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp().Result;
+        private readonly MasterOfCeremonies stubMasterOfCeremonies = new Stub().LoadApp()?.Result;
 
         Dictionary<Die, Face> DICE_N_FACES_1, DICE_N_FACES_2;
 
         public TurnTest()
         {
 
-            DICE_N_FACES_1 = (Dictionary<Die, Face>)stubMasterOfCeremonies.GameManager.GetAll().Result.First().GetHistory().First().DiceNFaces;
+            DICE_N_FACES_1 = (Dictionary<Die, Face>)stubMasterOfCeremonies.GameManager.GetAll()?.Result.First().GetHistory().First().DiceNFaces;
-            DICE_N_FACES_2 = (Dictionary<Die, Face>)stubMasterOfCeremonies.GameManager.GetAll().Result.Last().GetHistory().Last().DiceNFaces;
+            DICE_N_FACES_2 = (Dictionary<Die, Face>)stubMasterOfCeremonies.GameManager.GetAll()?.Result.Last().GetHistory().Last().DiceNFaces;
         }
 
         [Fact]

Sources/Tests/Model_UTs/Players/PlayerManagerTest.cs

@@ -111,15 +111,15 @@ namespace Tests.Model_UTs.Players
         }
 
         [Fact]
-        public async Task TestGetOneByNameIfValidButNotExistThenReturnNull()
+        public void TestGetOneByNameIfValidButNotExistThenReturnNull()
         {
             // Arrange
             PlayerManager playerManager = new();
             Player player = new("Bob");
-            await playerManager.Add(player);
+            playerManager.Add(player);
 
             // Act
-            Player result = await playerManager.GetOneByName("Clyde");
+            Player result = playerManager.GetOneByName("Clyde")?.Result;
 
             // Assert
             Assert.Null(result);
@@ -130,15 +130,15 @@ namespace Tests.Model_UTs.Players
         [InlineData("bob")]
         [InlineData("bob ")]
         [InlineData(" boB ")]
-        public async Task TestGetOneByNameIfValidThenReturnPlayer(string name)
+        public void TestGetOneByNameIfValidThenReturnPlayer(string name)
         {
             // Arrange
             PlayerManager playerManager = new();
             Player expected = new("Bob");
-            await playerManager.Add(expected);
+            playerManager.Add(expected);
 
             // Act
-            Player actual = await playerManager.GetOneByName(name);
+            Player actual = playerManager.GetOneByName(name)?.Result;
 
             // Assert
             Assert.Equal(expected, actual);