David
David

Reputation: 3056

Railroad oriented programming in clojure

I saw a talk about railroad oriented programming (https://www.youtube.com/watch?v=fYo3LN9Vf_M), but i somehow do not get how to work this out, if i use reduce, because reduce has two or even three arguments.

How am i able to to put the following code like a railroad? I seems to me hard, because of reduce taking a function as an argument in addition to the game object.

(defn play-game-reduce []

  (let [game-init
        (->>
         (io/initialize-cards-and-players)
         (shuffle-and-share-cards myio/myshuffle)
         (announce))

        play-round
        (reduce play-card (assoc-in game-init [:current-trick] '()) [:p1 :p2 :p3 :p4])]

    (reduce play-round game-init (range (get game-init :round-count)))))

The whole code is here: https://github.com/davidh38/doppelkopf/blob/master/src/mymain.clj

The code should more look like this:

(->> (io/initialize-cards-and-players)
             (shuffle-and-share-cards myio/myshuffle)
             (announce)
    reduce (play-round .. )
    reduce (play-card ...))

That would look to me much more explicit.

Upvotes: -1

Views: 244

Answers (2)

Martin Půda
Martin Půda

Reputation: 7576

That video was made for a different language and you can't directly transfer these ideas to Clojure.

I looked at your source code and there are some things to improve:

(defn play-card-inp []
  (eval (read-string (read-line))))

You shouldn't use eval in production code.

Read-string is unsafe and you should use clojure.edn/read-string instead. I'm not sure what is expected input here and what is the result of the evaluation, maybe you should use just clojure.edn/read here.

(defn myshuffle [cards]
  (shuffle cards)
  )

(defn initialize-cards-and-players []
  ; init cards
  (def cards '([0 :c], [1 :c],[2 :c], [3 :c], [0 :s], [1 :s], [2 :s], [3 :s]))
  (def players '(:p1 :p2 :p3 :p4))

  ;(def round-players (take 4 (drop (who-won_trick tricks) (cycle (keys players)))))
  ; mix and share cards
  {:players (zipmap players (repeat  {:cards () :tricks ()}))
   :current-trick ()
   :round-start-player :p1
   :cards cards
   :round-count (/ (count cards) (count players))
   :mode ""
   })

You should delete myshuffle and use directly shuffle where needed. Ending parenthesis shouldn't be on a separate line.

Don't use def (creates global variable) inside defn, use let (creates local variables). I would rewrite this as:

(defn new-deck []
  (for [letter [:c :s]
        number (range 4)]
    [number letter]))

(defn new-game []
  (let [cards (new-deck)
        players [:p1 :p2 :p3 :p4]]
    {:players (zipmap players (repeat {:cards () :tricks ()}))
     :current-trick ()
     :round-start-player :p1
     :cards cards
     :round-count (/ (count cards) (count players))
     :mode ""}))

Notes for mymain.clj:

(defn who-won-trick [trick]
  (eval (read-string (read-line))))

Some unused function, same problems as above.

(defn share-card-to-player [game players-cards]
  (assoc game
         :players
         (assoc
          (get game :players)
          (first players-cards)
          (assoc (get (game :players) (first players-cards))
                 :cards
                 (second players-cards)))))

Use assoc-in and some destructuring, something like this:

(defn share-card-to-player [game [player cards]]
  (assoc-in game [:players player :cards] cards))

Your next function:

(defn shuffle-and-share-cards [myshuffle game]
  (reduce share-card-to-player game
          (map vector
               (keys (get game :players))
               (->>  (get game :cards)
                     (myshuffle)
                     (partition (/ (count (get game :cards))
                                   (count (get game :players))))))))

You can also destructure hash-maps, so I would rewrite this as:

(defn shuffle-and-share-cards [{:keys [players cards] :as game}]
  (let [card-piles (->> cards
                        shuffle
                        (partition (/ (count cards)
                                      (count players))))]
    (reduce share-card-to-player game
            (map vector
                 (keys players)
                 card-piles))))

Next functions:

(defn announce [game]
  game)

(defn play-card [game curr-player]
  (println curr-player)
  (println game)

  (let [played-card (io/play-card-inp)]
    (->
     (assoc-in game [:players curr-player :cards]
               (remove #(= played-card %) (get-in game [:players curr-player :cards])))

     (assoc-in [:current-trick]
               (conj (game [:current-trick]) played-card)))))

announce is useless and update and update-in are better here:

(defn play-card [game curr-player]
  (println curr-player)
  (println game)
  (let [played-card (io/play-card-inp)]
    (-> game
        (update-in [:players curr-player :cards] #(remove #{played-card} %))
        (update :current-trick conj played-card))))

And finally, the last two functions:

(defn play-game-reduce []

  (let [game-init
        (->>
         (io/initialize-cards-and-players)
         (shuffle-and-share-cards myio/myshuffle)
         (announce))

        play-round
        (reduce play-card (assoc-in game-init [:current-trick] '()) [:p1 :p2 :p3 :p4])]

    (reduce play-round game-init (range (get game-init :round-count)))))

(defn play-game []

  (let [game-init
        (->>
         (io/initialize-cards-and-players)
         (shuffle-and-share-cards io/myshuffle)
         (announce))]

    (loop [round 1 game game-init]

      (let [game-next (loop [curr-player 1 game-next game]
                        (if (> curr-player 4)
                          game-next
                          (recur (inc curr-player)
                                 (play-card game-next (keyword (str "p" curr-player))))))]

        (if (> round 2)
          game-next
          (recur (inc round) game-next))))))

loop/recur will be probably more readable, but two reduce should also work:

(defn play-game-reduce []
  (let [game-init (-> (io/new-game)
                      shuffle-and-share-cards)]
    (reduce (fn [game round]
              (reduce play-card (assoc-in game [:current-trick] '()) [:p1 :p2 :p3 :p4]))
            game-init
            (range (get game-init :round-count)))))

(play-game-reduce)

Version with one reduce:

(defn play-game-reduce []
  (let [game-init (-> (io/new-game)
                      shuffle-and-share-cards)
        turns (for [round (range (:round-count game-init))
                    player [:p1 :p2 :p3 :p4]]
                [round player])]
    (reduce (fn [game [round player]]
              (let [state (cond-> game
                                  (= player (:round-start-player game)) (assoc-in [:current-trick] '()))]
                (play-card state player)))
            game-init
            turns)))

And I also noticed that there's no validation of whether the current player can really play inserted card.

Upvotes: 3

Alan Thompson
Alan Thompson

Reputation: 29986

OK, I watched the talk (for the record, it gives a 5 minute overview of FP, then discusses error handling in pipelines in F#.

I didn't really care for the content of the video.

Clojure uses Exceptions for error handling, so a Clojure function always has only one output. Therefore the whole bind and map thing in the video doesn't apply.

I haven't looked at F# much before, but after watching that video I think it over-complicates things without much benefit.

Upvotes: 1

Related Questions