Correct game logic

There is a case not covered by tests previously that was declaring a draw when it was instead a win for one of the player, only because it was the last turn in the game. Elm doesn't prevent one from writing bad code (ie: without legit' tests... ¯\_(ツ)_/¯). Ouch.
franckverrot · Feb 24, 2017 · 0d92898 · 0d92898
1 parent f66b6fa
commit 0d92898
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 20 deletions.
diff --git a/build/app.js b/build/app.js
@@ -9240,27 +9240,39 @@ var _user$project$Update$update = F2(
 					_user$project$Model$initialModel,
 					{ctor: '[]'});
 			case 'CheckWinner':
-				var _p5 = _p2._0;
+				var _p6 = _p2._0;
 				if (_user$project$GameLogic$noneUnclaimed(model.boxes)) {
-					return A2(
-						_elm_lang$core$Platform_Cmd_ops['!'],
-						_elm_lang$core$Native_Utils.update(
-							model,
-							{
-								winner: _elm_lang$core$Maybe$Just(_user$project$Player$Unclaimed)
-							}),
-						{ctor: '[]'});
+					var _p3 = A2(_user$project$Update$playerWon, _p6, model);
+					if (_p3.ctor === 'Ok') {
+						return A2(
+							_elm_lang$core$Platform_Cmd_ops['!'],
+							_elm_lang$core$Native_Utils.update(
+								model,
+								{
+									winner: _elm_lang$core$Maybe$Just(_p3._0)
+								}),
+							{ctor: '[]'});
+					} else {
+						return A2(
+							_elm_lang$core$Platform_Cmd_ops['!'],
+							_elm_lang$core$Native_Utils.update(
+								model,
+								{
+									winner: _elm_lang$core$Maybe$Just(_user$project$Player$Unclaimed)
+								}),
+							{ctor: '[]'});
+					}
 				} else {
-					var _p3 = _p2._1;
-					if (_p3 === true) {
-						var _p4 = A2(_user$project$Update$playerWon, _p5, model);
-						if (_p4.ctor === 'Ok') {
+					var _p4 = _p2._1;
+					if (_p4 === true) {
+						var _p5 = A2(_user$project$Update$playerWon, _p6, model);
+						if (_p5.ctor === 'Ok') {
 							return A2(
 								_elm_lang$core$Platform_Cmd_ops['!'],
 								_elm_lang$core$Native_Utils.update(
 									model,
 									{
-										winner: _elm_lang$core$Maybe$Just(_p4._0)
+										winner: _elm_lang$core$Maybe$Just(_p5._0)
 									}),
 								{ctor: '[]'});
 						} else {
@@ -9269,7 +9281,7 @@ var _user$project$Update$update = F2(
 								_elm_lang$core$Native_Utils.update(
 									model,
 									{
-										currentPlayer: _user$project$Update$changePlayer(_p5)
+										currentPlayer: _user$project$Update$changePlayer(_p6)
 									}),
 								{ctor: '[]'});
 						}

diff --git a/src/Update.elm b/src/Update.elm
@@ -55,7 +55,11 @@ update msg model =
 
     CheckWinner player currentPlayerShouldChange _ ->
       if noneUnclaimed model.boxes then
-        { model | winner = Just Unclaimed } ! []
+        case playerWon player model of
+          -- There's a winner
+          Ok player -> { model | winner = Just player         } ! []
+          -- it's a draw
+          Err _     -> { model | winner = Just Unclaimed } ! []
 
       else
         case currentPlayerShouldChange of

diff --git a/tests/unit/UpdateTests.elm b/tests/unit/UpdateTests.elm
@@ -15,36 +15,59 @@ updateTests = [ describe "Update.update"
                       \() ->
                           Expect.equal (initialModel ! [])
                             <| update Reset initialModel
-                  , test "Handles CheckWinner -- none unclaimed" <|
+
+                  , test "Handles CheckWinner -- none unclaimed -- and there's a winner" <|
+                      \() ->
+                        let
+                            message       = (CheckWinner A False Time.second)
+                            previousModel = { initialModel
+                                            | boxes = (Array.fromList [ A, A, A
+                                                                      , A, B, B
+                                                                      , B, B, A ])
+                                            }
+                            (newModel, _) = update message previousModel
+                        in
+                            Expect.equal (Just A) (newModel.winner)
+
+                  , test "Handles CheckWinner -- none unclaimed -- no winner" <|
                       \() ->
                         let
                             message       = (CheckWinner A False Time.second)
                             previousModel = { initialModel
-                                            | boxes = (Array.fromList [A, A, A, A, B, B, B, A, B])
+                                            | boxes = (Array.fromList [ A, A, B
+                                                                      , B, B, A
+                                                                      , A, B, A ])
                                             }
                             (newModel, _) = update message previousModel
                         in
                             Expect.equal (Just Unclaimed) (newModel.winner)
+
                   , test "Handles CheckWinner -- legal move with winner" <|
                       \() ->
                         let
                             message       = (CheckWinner A True Time.second)
                             previousModel = { initialModel
-                                            | boxes = (Array.fromList [A, A, A, B, B, Unclaimed, Unclaimed, Unclaimed, Unclaimed])
+                                            | boxes = (Array.fromList [ A, A, A
+                                                                      , B, B, Unclaimed
+                                                                      , Unclaimed, Unclaimed, Unclaimed])
                                             }
                             (newModel, _) = update message previousModel
                         in
                             Expect.equal (Just A) newModel.winner
+
                   , test "Handles CheckWinner -- legal move with no winner" <|
                       \() ->
                         let
                             message       = (CheckWinner A True Time.second)
                             previousModel = { initialModel
-                                            | boxes = (Array.fromList [A, A, Unclaimed, B, B, Unclaimed, Unclaimed, Unclaimed, Unclaimed])
+                                            | boxes = (Array.fromList [ A, A, Unclaimed
+                                                                      , B, B, Unclaimed
+                                                                      , Unclaimed, Unclaimed, Unclaimed])
                                             }
                             (newModel, _) = update message previousModel
                         in
                             Expect.equal Nothing newModel.winner
+
                   , test "Handles CheckWinner -- illegal move" <|
                       \() ->
                         let