Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX #4

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
7 changes: 5 additions & 2 deletions spec/clojure_ttt/board_spec.clj
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@
(it "should know if there is a draw"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You use the words should know a lot, I would either simplify that to knows or determines
e.g. (it 'determines if a position is valid')

(should= true (draw? ["x"])))

(it "should know valid positions"
(should= false (valid-position? ["x"] 0)))
(it "should know if a position is valid"
(should= true (valid-position? ["x" "-"] 1)))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use the function should and should-not rather than should= when things are just true/false


(it "should know if a position is not valid"
(should= false (valid-position? ["x" "-"] 0)))

(it "should know all available positions"
(should= [1 2] (available-positions ["x" "-" "-"])))
Expand Down
2 changes: 1 addition & 1 deletion spec/clojure_ttt/game_spec.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
(it "should make a move"
(should= ["o" "x" "-"]
(with-in-str "1"
(make-move ["o" "-" "-"]))))
(make-move ["o" "-" "-"] "x"))))

(it "does not let you make moves in invalid cells"
(should= ["-"]
Expand Down
22 changes: 22 additions & 0 deletions spec/clojure_ttt/presenter_spec.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(ns clojure-ttt.presenter-spec
(:require [speclj.core :refer :all]
[clojure-ttt.presenter :refer :all]))

(describe "presenter"
(it "displays gamemode options"
(should-contain "Type 0 to play human vs human"
(with-out-str (show-gamemode-options))))

(it "displays the board correctly"
(should-contain "x--\nx--\n---"
(with-out-str (show-board ["x" "-" "-" "x" "-" "-" "-" "-" "-"]))))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use keywords to represent marks, rather than strings.
Strings have individual areas of memory, whereas keywords share areas, like constants. I.e. "x" "x" takes up 2 unit of memory, whereas :x :x takes up just 1 unit.


(it "displays the winner"
(should-contain "x has won the game!"
(with-out-str (show-round-message ["x" "x" "x" "o" "-" "-" "-" "o" "o"]))))

(it "displays if its a draw"
(should-contain "draw"
(with-out-str (show-round-message ["x" "x" "o"
"o" "x" "x"
"x" "o" "o"])))))
4 changes: 2 additions & 2 deletions src/clojure_ttt/board.clj
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@
(true? (some #(has-win? %) (winning-lines board (size board)))))

(defn draw? [board]
(not (some #{"-"} board)))
(not-any? #{"-"} board))

(defn available-positions [board]
(->> (zipmap (iterate inc 0) board)
(filter (fn [[position cell]] (= cell "-")))
(map (fn [[position cell]] position))))

(defn valid-position? [board position]
(contains? (vec (available-positions board)) position))
(true? (some #(= position %) (available-positions board))))

(defn find-turn [board]
(if (even? (count (available-positions board))) "x" "o"))
Expand Down
2 changes: 1 addition & 1 deletion src/clojure_ttt/computer.clj
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@
(all-rounds-played board player (available-positions board)))))

(defn find-best-move [board player]
(:move (minimax board player 8 ["x" "o"])))
(:move (minimax board player (min 8 (count (available-positions board))) ["x" "o"])))
37 changes: 30 additions & 7 deletions src/clojure_ttt/game.clj
Original file line number Diff line number Diff line change
@@ -1,22 +1,45 @@
(ns clojure-ttt.game
(use [clojure-ttt.board]))
(use [clojure-ttt.board]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use use. Prefer (refer [clojure-ttt.board] :as board)

[clojure-ttt.computer]
[clojure-ttt.presenter]))

(defn game-over? [board]
(or (draw? board) (any-wins? board)))

(defn- read-move []
(read-string (read-line)))

(defn make-move [board]
(defn make-move [board player]
(let [move (read-move)]
(cond
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an if statement I think?

(valid-position? board move) (mark-board board move (find-turn board))
(valid-position? board move) (mark-board board move player)
:else board)))

(defn start-game [board]
(println board)
(defn make-move-computer [board player]
(mark-board board (find-best-move board player) player))

(defn play-until-over [board gamemode]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are the tests for this new functionality?

(show-round-message board)
(show-board board)
(when-not (game-over? board)
(recur (make-move board))))
(let [turn (find-turn board)]
(cond
(and (= gamemode :hvc) (= turn "o")) (recur (make-move-computer board turn) gamemode)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

game-mode

(= gamemode :cvc) (recur (make-move-computer board turn) gamemode)
:else (recur (make-move board turn) gamemode)))))

(defn- ask-for-settings [board]
(show-gamemode-options)
(let [input (read-move)]
(cond
(= input 1) (play-until-over board :hvc)
(= input 2) (play-until-over board :cvc)
:else (play-until-over board :hvh))))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about invalid entries? Where should the responsibility for validation go?


(defn ask-to-play-again []
(show-options)
(if (= 1 (read-move)) (ask-for-settings (create-board 3))))

(defn -main [& args]
(start-game (create-board 3)))
(ask-for-settings (create-board 3))
(ask-to-play-again))
29 changes: 29 additions & 0 deletions src/clojure_ttt/presenter.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
(ns clojure-ttt.presenter
(use [clojure-ttt.board]))

(defn- clear-console []
(println "\033[H\033[2J"))

(defn- welcome-user []

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no spec for this

(println "\033[35mWelcome! Please select a gamemode.")
(println "------------------------------------------"))

(defn show-gamemode-options []
(clear-console)
(welcome-user)
(println "\033[36mType 0 to play human vs human. \n1 to play vs a computer. \n2 to watch two computers play."))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not clear when displayed in the console. Probably because of the word 'type' being either a verb or a noun.. You could lose 'type' altogether and have:
0 - to play human vs human
1 - to play ....


(defn show-board [board]
(let [partitioned-board (map #(apply str %) (partition 3 board))]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 3 here is breaking encapsulation. Create a board-size function in your board namespace and call that instead. Your display functions should not know the size of the board.

(print "\033[31m")
(println (apply str(interpose "\n" partitioned-board)))))

(defn show-round-message [board]
(println (str "\033[35m" "--------------------------"))
(cond
(any-wins? board) (println (str (last-move board) " has won the game!"))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These println calls can be dragged outside the cond

(draw? board) (println "The game is a draw!")
:else (println (str (find-turn board) " please make your move."))))

(defn show-options []
(println "Press 1 to play again, anything else to quit."))