Idan domain ucc03 0 #24

idanizi · 2017-05-21T22:40:57Z

UCC03 - Rooms Management - Domain Layer

…ment

- bank and wallet extend same abstruct class. - todo: adapt bank to this change. - implementing features to game center, room, player. related to 'join next hand'

…nter a singleton

…ervice layer), controller(comm layer)

…ost of them related to not meeded MVC

…val)

…st thig!

# Conflicts: # Poker.BE/Poker.BE.Domain.Tests/Game/PlayerTests.cs # Poker.BE/Poker.BE.Domain.Tests/Game/RoomTests.cs # Poker.BE/Poker.BE.Domain/Game/Player.cs # Poker.BE/Poker.BE.Domain/Game/Room.cs # Poker.BE/Poker.BE.Domain/Game/Round.cs # Poker.BE/Poker.BE.Domain/Game/Turn.cs # Poker.BE/Poker.BE.Domain/Game/Wallet.cs # Poker.BE/Poker.BE.Domain/Poker.BE.Domain.csproj # Poker.BE/Poker.BE.Domain/Utility/Exceptions/NotEnoughMoneyException.cs # Poker.BE/Poker.BE.Domain/Utility/Exceptions/NotEnoughPlayersException.cs

tomer314

basically everything is fine, just see my comments/questions.
those are small changes.

tomer314 · 2017-05-22T23:57:39Z

Poker.BE/Poker.BE.Domain/Game/Room.cs

+            this.CurrentHand = null;
+            this.Name = new GameConfig().Name;
+        }
+
        public Player CreatePlayer()
        {
            var result = new Player();


the player suppose to have a name as I understood.
so maybe the CreatePlayer() method should get his name from the user's name?

Mmm.. yeah i thought about it. It because of the equals override i did. I can add a condition to the method that if the name is empty, do a shallow compare instead of deep. It should solve the problem. I'll add a test for that

tomer314 · 2017-05-22T23:59:48Z

Poker.BE/Poker.BE.Domain/Core/GameCenter.cs

        {
-            // TODO
-            throw new NotImplementedException();
+            creator = new Player();


player should get the name from the user's name?

No. Player has a nickname. User has a name. They can be edited separately

tomer314 · 2017-05-23T00:12:17Z

Poker.BE/Poker.BE.Domain/Game/Chair.cs

-        #region Constants
-        public static readonly int NCHAIRS_IN_ROOM = 10;
-        #endregion
+

        #region Fields
        private int index;
        private bool isBusy;


please change it to "isTaken"...
it's driving me crazy ;P

tomer314 · 2017-05-23T00:28:03Z

Poker.BE/Poker.BE.Domain/Game/GameConfig.cs

+        private double _minimumBet;
+        private string _name;
+        private GamePreferences _preferences;
+        #endregion


You're missing the "chip policy" option that tells you how many chips do the player gets when joining the game.
this is NOT related to buy-in policy... it's a different thing

I leave it to 'game preferences' as according to our design

tomer314 · 2017-05-23T00:29:11Z

Poker.BE/Poker.BE.Domain/Game/GameConfig.cs

@@ -7,32 +7,199 @@ namespace Poker.BE.Domain.Game
    public class GameConfig
    {

+        #region Fields
+        private int _maxNumberOfActivePlayers;
+        private int _maxNumberOfPlayers;


we care only about the min/max number of players at the table, aka "active players"

No. Min/max numbers are for room configuration. Active Players is a property in Room class. Updated according to room state

tomer314 · 2017-05-23T00:36:03Z

Poker.BE/Poker.BE.Domain/Game/GameConfig.cs

        /// </summary>
-        public int MinNumberOfPlayers { get; set; }
+        public int MaxNumberOfPlayers


can remove this MaxNumberOfPlayers property

No. We need it

tomer314 · 2017-05-23T00:52:12Z

Poker.BE/Poker.BE.Domain/Game/Player.cs

        #region Properties
        public State CurrentState { get; set; }
+        public Wallet Wallet { get { return _wallet; } }
+        public double WalletValue { get { return _wallet.Value; } private set { _wallet.Value = value; } }


we need to choose:
we have the amountOfMoney field in the Wallet class and we have the value field in its father class.
we don't need both

Yup. Issue #25 wallet

tomer314 · 2017-05-23T00:56:05Z

Poker.BE/Poker.BE.Domain/Game/Room.cs

+            get { return config.BuyInCost; }
+            set { config.BuyInCost = value; }
+        }
+        #endregion


do we need all those properties? doesn't it enough to have the GamePrefences object related to it that holds all of this information?

By UC of create room.. yup, we do need them

tomer314 · 2017-05-23T01:09:47Z

Poker.BE/Poker.BE.Domain/Utility/MoneyStorage.cs

+
+        #endregion
+    }
+}


I believe we don't need this class at all...
let's use just one kind of currency.
the chip-to-money conversion will be calculated by the game preferences in the specific room (just divide the chip policy by the buy-in policy)

I differ. :) Let's talk on this f2f

This related to #25

Good job here idan! Can you explain me too what do we need this class?

@arieltobiana see Issue #25 wallet

tomer314 · 2017-05-23T01:11:13Z

Poker.BE/Poker.BE.Domain/Game/Wallet.cs

@@ -6,16 +6,21 @@

 namespace Poker.BE.Domain.Game
 {
-    public class Wallet
+    public class Wallet : Utility.MoneyStorage


need to decide if we need the MoneyStorage class.
see comment in that class

arieltobiana · 2017-05-23T10:47:18Z

Poker.BE/Poker.BE.API/Controllers/RoomController.cs

+        #region Constructors
+        public RoomController()
+        {
+            service = new Service.Services.RoomsService();


new Service.Services? seems like bad smell or maybe I miss something

you miss something.

arieltobiana · 2017-05-23T10:48:39Z

Poker.BE/Poker.BE.Domain.Tests/Core/GameCenterTests.cs

+        [TestInitialize]
+        public void Before()
+        {
+            gameCenter = GameCenter.Instance;


gamecenter is singleton? nice good idea

yes. as we design. credit for all.

arieltobiana · 2017-05-23T10:50:07Z

Poker.BE/Poker.BE.Domain.Tests/Core/GameCenterTests.cs

+            var expRoom2 = gameCenter.CreateNewRoom(level, config, out creator2).Name = "test room 2";
+
+            //Act
+            var actual = gameCenter.FindRoomsByCriteria(25);


what is criteria 25?

integer bind to level parameter - look at the method definition

arieltobiana · 2017-05-23T10:51:16Z

Poker.BE/Poker.BE.Domain.Tests/Core/GameCenterTests.cs

+            Assert.AreEqual(0, expRoom.ActivePlayers.Count);
+        }
+    }
+}


Very good tests! 👍

arieltobiana · 2017-05-23T10:51:55Z

Poker.BE/Poker.BE.Domain.Tests/Core/UserTests.cs

+        //{
+
+        //}
+


Maybe can we delete comments

@arieltobiana - this tests are for Connect() Disconnect() methods of User class. that you didn't make any test for them and no documentation that explain what are their purpose. look at the //TODO: ...

arieltobiana · 2017-05-23T11:11:24Z

Poker.BE/Poker.BE.Domain/Game/Round.cs

-                            RaisePreconditions(this.CurrentPlayer.Wallet.amountOfMoney + LiveBets[CurrentPlayer] - TotalRaise);
-                            Raise(this.CurrentPlayer.Wallet.amountOfMoney + LiveBets[CurrentPlayer] - TotalRaise);
+                            RaisePreconditions(this.CurrentPlayer.Wallet.AmountOfMoney + LiveBets[CurrentPlayer] - TotalRaise);
+                            Raise(this.CurrentPlayer.Wallet.AmountOfMoney + LiveBets[CurrentPlayer] - TotalRaise);



Mabrouk about the code convention 👍

arieltobiana · 2017-05-23T11:12:42Z

Poker.BE/Poker.BE.Domain/Game/Round.cs

                    newPartialPot.PlayersClaimPot.Add(player);
+                }
+
                lastPartialPot.PlayersClaimPot.Remove(CurrentPlayer);

                newPartialPot.AmountToClaim = amountToAdd + lastPlayerCurrentBet;
                lastPartialPot.AmountToClaim -= newPartialPot.AmountToClaim;
                newPartialPot.Value = lastPartialPot.Value - (lastPartialPot.AmountToClaim * lastPartialPot.PlayersClaimPot.Count);
                lastPartialPot.Value = lastPartialPot.AmountToClaim * lastPartialPot.PlayersClaimPot.Count;



I'm not sure that I understand the link between pot and partialpot. Do we have some pot tests?

@tomer314 - the pot expert 😆

arieltobiana · 2017-05-23T11:13:38Z

Poker.BE/Poker.BE.Domain/Utility/AddressComparer.cs

+            return obj.GetHashCode();
+        }
+    }
+}


@idanizi what is the purpose of this class?

i've added ///<summery>

arieltobiana · 2017-05-23T11:15:17Z

Poker.BE/Poker.BE.Domain/Utility/MoneyStorage.cs

+
+        #endregion
+    }
+}


Good job here idan! Can you explain me too what do we need this class?

arieltobiana · 2017-05-23T11:16:12Z

Poker.BE/Poker.BE.Service/Services/RoomsService.cs

+        }
+        #endregion
+
+    }// class
 }


Very good job! I'm impressed!

idanizi

Answered & Done

i've pass through @tomer314 and @arieltobiana reviews - many thanks!

idanizi · 2017-05-23T13:22:57Z

Poker.BE/Poker.BE.Domain/Utility/AddressComparer.cs

+            return obj.GetHashCode();
+        }
+    }
+}


i've added ///<summery>

idanizi · 2017-05-23T13:24:02Z

Poker.BE/Poker.BE.Domain/Utility/MoneyStorage.cs

+
+        #endregion
+    }
+}


@arieltobiana see Issue #25 wallet

idanizi · 2017-05-23T13:24:22Z

Poker.BE/Poker.BE.Service/Services/RoomsService.cs

+        }
+        #endregion
+
+    }// class
 }


idanizi · 2017-05-23T13:25:24Z

Poker.BE/Poker.BE.API/Controllers/RoomController.cs

+        #region Constructors
+        public RoomController()
+        {
+            service = new Service.Services.RoomsService();


you miss something.

idanizi · 2017-05-23T13:26:02Z

Poker.BE/Poker.BE.Domain.Tests/Core/GameCenterTests.cs

+        [TestInitialize]
+        public void Before()
+        {
+            gameCenter = GameCenter.Instance;


yes. as we design. credit for all.

idanizi · 2017-05-23T13:45:10Z

Poker.BE/Poker.BE.Domain/Game/Player.cs

+            {
+                throw new PlayerModeException("Unable to stand up: Player already a spectator.");
+            }
+


❗️ ❗️ ❗️ this is "leave room" not "stand up". make it as a SPIKE at trello. we dont have this UC!

idanizi · 2017-05-23T13:48:05Z

Poker.BE/Poker.BE.Domain/Game/Room.cs

            deck = new Deck();
-            chairs = new Chair[Chair.NCHAIRS_IN_ROOM];
+            chairs = new Chair[NCHAIRS_IN_ROOM];



idanizi · 2017-05-23T13:49:09Z

Poker.BE/Poker.BE.Domain/Game/Room.cs

@@ -98,9 +173,50 @@ public Room(Player creator, GamePreferences preferences) : this(creator)
            Preferences = preferences;
        }

+        public Room(Player creator, GameConfig config) : this(creator, config.Preferences)
+        {


just for the creation, to add it to "passive players". he doesn't have spacial privileges

idanizi · 2017-05-23T14:36:29Z

Poker.BE/Poker.BE.Domain/Game/Room.cs

+            this.CurrentHand = null;
+            this.Name = new GameConfig().Name;
+        }
+
        public Player CreatePlayer()
        {
            var result = new Player();


idanizi · 2017-05-23T14:39:10Z

Poker.BE/Poker.BE.Domain/Game/Round.cs

                    newPartialPot.PlayersClaimPot.Add(player);
+                }
+
                lastPartialPot.PlayersClaimPot.Remove(CurrentPlayer);

                newPartialPot.AmountToClaim = amountToAdd + lastPlayerCurrentBet;
                lastPartialPot.AmountToClaim -= newPartialPot.AmountToClaim;
                newPartialPot.Value = lastPartialPot.Value - (lastPartialPot.AmountToClaim * lastPartialPot.PlayersClaimPot.Count);
                lastPartialPot.Value = lastPartialPot.AmountToClaim * lastPartialPot.PlayersClaimPot.Count;



@tomer314 - the pot expert 😆

idanizi added 30 commits May 13, 2017 21:37

start working on the tests of game center

9ede6f5

start working on the tests of game center

a5f0916

leagues level, enter a room. create a room

421ce80

room creation

004f14b

adding 2 exceptionsworking on gamecenter, game config min/max enforce…

bdcdfc9

…ment

join next hand

ef7fc46

wallet:

9b45063

- bank and wallet extend same abstruct class. - todo: adapt bank to this change. - implementing features to game center, room, player. related to 'join next hand'

uc013 stand up from table

7114134

working on user exposing methods of gamecenter. todo: make the gamece…

406e5ec

…nter a singleton

game center is now a singleton. logger is thread safe (tests pass :) )

780b9a5

user.enter room

4d402ff

player test - pass

5cbaf57

player test - pass

6547ec0

service layer example

62ba035

renaming NewRoom request / result to CreateNewRoom...

3d93394

service layer - to add - user not found exception

55087d1

working on the service and communication layer - ucc03 room service(s…

20017a4

…ervice layer), controller(comm layer)

clearing some uneeded files at Poker.API, todo: need to clear more. m…

a56b784

…ost of them related to not meeded MVC

initial room controller, todo excemption catching

c6fd335

room service and controller

a434bec

setting Newtonsoft.Json as the json formatter (and xml formatter remo…

a9c149a

…val)

testing room

49cd55f

editing todos, testing room and bug fixing

3b9f3ef

fixing game config

dd99c53

room(:player) ctor test pass

eed1722

room(:player, :preferences) ctor test pass

a188238

working on room(:player, :gameconfig) ctor test

049640b

working on room(:player, :gameconfig) ctor test

0ce05e4

room(:player, :gameconfig) ctor test pass

bf1f9db

join player to table - room - pass

e33952e

idanizi added 17 commits May 20, 2017 20:12

create player - room - pass

06c3e7c

take and leave chair - room - pass

888637a

enter room & create new room - gamecenter - pass

c45618d

working on join next hand tests

6fc5e29

working on find rooms

7475af0

working on find rooms

717aaac

working on find rooms

eff02ed

find room by criteria - gamecenter - level, palyer - pass

017e206

gamecenter test all pass. working on user tests of ucc03 methods - la…

12edd8f

…st thig!

gamecenter test all pass. working on user tests of ucc03 methods - la…

053b018

…st thig!

all tests pass ucc03

b34ea9e

fixing for ci

e2258b7

fixing for ci

e71afa6

fixing for ci

18b0732

fixing for ci

be9f021

fixing build bugs after merge. todo - paly move test not fail - fixme

29c6e5d

idanizi mentioned this pull request May 22, 2017

wallet #25

Open

all tests pass

218bb3a

idanizi requested review from tomer314, Asafb26 and wainergal May 22, 2017 23:41

tomer314 approved these changes May 23, 2017

View reviewed changes

arieltobiana reviewed May 23, 2017

View reviewed changes

idanizi added 2 commits May 23, 2017 17:03

fix for PR comments

5193a2d

fix for PR comments - all test pass

fe633eb

idanizi commented May 23, 2017

View reviewed changes

idanizi merged commit 86a229c into master May 23, 2017

idanizi mentioned this pull request May 25, 2017

Gal service 3 #26

Merged

Idan domain ucc03 0 #24

Idan domain ucc03 0 #24

Conversation

idanizi commented May 21, 2017 • edited Loading

UCC03 - Rooms Management - Domain Layer

tomer314 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idanizi left a comment

Choose a reason for hiding this comment

Answered & Done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idanizi commented May 21, 2017 •

edited

Loading