UX #4
UX #4
Conversation
| (defn make-move-computer [board player] | ||
| (mark-board board (find-best-move board player) player)) | ||
|
|
||
| (defn play-until-over [board gamemode] |
There was a problem hiding this comment.
Where are the tests for this new functionality?
| (defn- clear-console [] | ||
| (println "\033[H\033[2J")) | ||
|
|
||
| (defn- welcome-user [] |
| (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.")) |
There was a problem hiding this comment.
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 ....
| (cond | ||
| (= input 1) (play-until-over board :hvc) | ||
| (= input 2) (play-until-over board :cvc) | ||
| :else (play-until-over board :hvh)))) |
There was a problem hiding this comment.
What about invalid entries? Where should the responsibility for validation go?
| (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))) |
There was a problem hiding this comment.
I think you can use the function should and should-not rather than should= when things are just true/false
|
|
||
| (it "displays the board correctly" | ||
| (should-contain "x--\nx--\n---" | ||
| (with-out-str (show-board ["x" "-" "-" "x" "-" "-" "-" "-" "-"])))) |
There was a problem hiding this comment.
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.
|
|
||
| (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 |
| (recur (make-move board)))) | ||
| (let [turn (find-turn board)] | ||
| (cond | ||
| (and (= gamemode :hvc) (= turn "o")) (recur (make-move-computer board turn) gamemode) |
| (println "\033[36mType 0 to play human vs human. \n1 to play vs a computer. \n2 to watch two computers play.")) | ||
|
|
||
| (defn show-board [board] | ||
| (let [partitioned-board (map #(apply str %) (partition 3 board))] |
There was a problem hiding this comment.
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.
| (defn show-round-message [board] | ||
| (println (str "\033[35m" "--------------------------")) | ||
| (cond | ||
| (any-wins? board) (println (str (last-move board) " has won the game!")) |
There was a problem hiding this comment.
These println calls can be dragged outside the cond
| @@ -1,22 +1,45 @@ | |||
| (ns clojure-ttt.game | |||
| (use [clojure-ttt.board])) | |||
| (use [clojure-ttt.board] | |||
There was a problem hiding this comment.
Don't use use. Prefer (refer [clojure-ttt.board] :as board)
| @@ -29,8 +29,11 @@ | |||
| (it "should know if there is a draw" | |||
There was a problem hiding this comment.
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')
|
While the overall Clojure code is clean, you still need to work on TDD. It's very noticeable when tests are written after (or not at all). E.g. |
No description provided.