CODEWITHSUNDEEP
clojure
loloof64
loloof64

Reputation: 5402

Function in order to show a TicTacToe board : can it be simplified and more readable?

As I am learning clojure, I am trying to build a simple tic tac toe game, without ia. I started with a method in order to show a board, but it seems very ugly to me : i am using internal functions, in order to make them local to the show-board method, so that it can't be instantiated outside. Perhaps the reason of the bad looking.

Here is the function (which works as I want) :

(defn show-board [board]
    "Prints the board on the screen.
     Board must be a list of three lists of three elements with
     :cross for a cross and :circle for a circle, other value for nothing"
    (let [convert-elem (fn [elem] 
                            (cond
                                    (= elem :cross) "X"
                                    (= elem :circle) "O"
                                    :other "_"))
          convert-line (fn [elems-line]
                            (reduce str (map convert-elem elems-line)))]
         (doseq [line board]
            (println (convert-line line)))))

Here is a use case :

(show-board (list (list :cross :circle :none) (list :none :circle :none) (list :cross :none :none)))

Sorry for the ugly code, this is because I come from Java, and I am starting in Clojure. (I think I will really get benefits from learning Clojure, and make games with it, so I can't just leave it).

Another reason why I want to simplify it is for code maintenance and readability.

Thanks in advance

Upvotes: 1

Views: 221

Answers (3)

cgrand
cgrand

Reputation: 7949

What about simply letting the underlying writer to buffer the output rather than creating intermediate strings (even if you create them through a StringBuilder)?

=> (defn show-board [board]
     (doseq [line board]
       (doseq [item line]
         (print ({:cross \X :circle \O} item \_)))
       (newline)))

And if you want to get a string rather than print them out, just use with-out-str.

Upvotes: 2

noisesmith
noisesmith

Reputation: 20194

(defn show-board
  [board]
  (let [convert (fn [el] (get {:cross \X :circle \O} el \_))]
    (doseq [line board]
      (doseq [el line]
        (print (convert el)))
      (println))))

Upvotes: 2

Michał Marczyk
Michał Marczyk

Reputation: 84351

Using internal functions is perfectly fine, though in this case using letfn would perhaps look better. Additionally, convert-elem can be simplified with case and convert-line should use apply rather than reduce for reasons I explained in my answer to the Clojure: reduce vs. apply SO question (in short, with apply a single StringBuilder is used and the resulting process is linear; with reduce, each step involves allocating a new StringBuilder and the process is quadratic; this doesn't make much difference is a small case like this, but it is nonetheless better style to use the correct approach).

Here's the modified function:

(defn show-board [board]
  "Prints the board on the screen.
   Board must be a list of three lists of three elements with
   :cross for a cross and :circle for a circle, other value for
   nothing."
  (letfn [(convert-elem [elem] 
            (case elem
              :cross "X"
              :circle "O"
              "_"))
          (convert-line [elems-line]
            (apply str (map convert-elem elems-line)))]
    (doseq [line board]
      (println (convert-line line)))))

As a side note, this function actually takes an arbitrary seqable of seqables, not necessarily a list of lists. A more usual concrete type choice in Clojure would be to use vectors:

user> (show-board [[:cross :circle :none]
                   [:none :circle :none]
                   [:cross :none :none]])
XO_
_O_
X__
nil

Upvotes: 2

Related Questions