HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Higher-order FizzBuzz in Clojure

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
orderclojurehigherfizzbuzz

Problem

Help me make it better. Requirement: support arbitrary number of factors, not just 3 & 5.

(ns fizzbuzz.core
  (:use [clojure.contrib.string :only (join)]))

(def targets (sorted-map 3 "fizz" 5 "buzz" 7 "baz"))

;; match any applicable factors
(defn match [n, factors]
  (filter #(= 0 (mod n %)) factors))

(defn fizzbuzz [n]
  (let [factors (keys targets)
        matches (match n factors)]
    (if (= 0 (count matches))
      (str n)
      (reduce str (map targets matches)))))

(join ", " (map fizzbuzz (range 1 20)))

Solution

I tried keeping your approach, although I would've made it an infinite lazy sequence. Yours is probably a better idea, since you still have the infinite sequence (mapping to range as you did) and you can map it to other non-linear sequences too.

(def targets  ;; This is more readable for me
  (sorted-map
    3 "fizz",
    5 "buzz",
    7 "baz"))

(defn fizzbuzz [targets n]  ;; Pass the targets here! Commas aren't very common in args lists
  (let [matches (keep (fn [[f w]]  ;; Destructuring is nice :P
                        (when (zero? (mod n f)) w))  ;; Got rid of your helper function
                      targets)]  ;; ^ Use 'zero?' instead of '(= 0 ...)'
    (if (empty? matches)  ;; Use 'empty?' instead of '(= 0 (count ...))'
      (str n)
      (apply str matches))))  ;; I prefer apply here, but seen both

;;;;; EXAMPLE
(map (partial fizzbuzz targets) (range 1 20))
   => ("1" "2" "fizz" "4" "buzz" "fizz" "baz" "8" "fizz" "buzz" "11" "fizz" "13" "baz" "fizzbuzz" "16" "17" "fizz" "19" "buzz")


There's an even more idiomatic way to rewrite the (if (empty? matches)...) bit:

(if matches
  (apply str matches)
  (str n))


Lazy sequence style. Consider fizzbuzz and targets are already defined:

(defn lazy-fizzbuzz
  ([targets n]
    (lazy-seq
      (cons (fizzbuzz targets n)
            (lazy-fizzbuzz targets (inc n)))))
  ([targets]
    (lazy-fizzbuzz targets 1)))

;;;;; EXAMPLES
(take 20 (lazy-fizzbuzz targets))
   => ("1" "2" "fizz" "4" "buzz" "fizz" "baz" "8" "fizz" "buzz" "11" "fizz" "13" "baz" "fizzbuzz" "16" "17" "fizz" "19" "buzz")

;; You can also specify an initial n
(take 20 (lazy-fizzbuzz targets 20))
   => ("buzz" "fizzbaz" "22" "23" "fizz" "buzz" "26" "fizz" "baz" "29" "fizzbuzz" "31" "32" "fizz" "34" "buzzbaz" "fizz" "37" "38" "fizz")

;; which is the same as...
(take 20 (drop 19 (lazy-fizzbuzz targets)))
   => ("buzz" "fizzbaz" "22" "23" "fizz" "buzz" "26" "fizz" "baz" "29" "fizzbuzz" "31" "32" "fizz" "34" "buzzbaz" "fizz" "37" "38" "fizz")

Code Snippets

(def targets  ;; This is more readable for me
  (sorted-map
    3 "fizz",
    5 "buzz",
    7 "baz"))

(defn fizzbuzz [targets n]  ;; Pass the targets here! Commas aren't very common in args lists
  (let [matches (keep (fn [[f w]]  ;; Destructuring is nice :P
                        (when (zero? (mod n f)) w))  ;; Got rid of your helper function
                      targets)]  ;; ^ Use 'zero?' instead of '(= 0 ...)'
    (if (empty? matches)  ;; Use 'empty?' instead of '(= 0 (count ...))'
      (str n)
      (apply str matches))))  ;; I prefer apply here, but seen both

;;;;; EXAMPLE
(map (partial fizzbuzz targets) (range 1 20))
   => ("1" "2" "fizz" "4" "buzz" "fizz" "baz" "8" "fizz" "buzz" "11" "fizz" "13" "baz" "fizzbuzz" "16" "17" "fizz" "19" "buzz")
(if matches
  (apply str matches)
  (str n))
(defn lazy-fizzbuzz
  ([targets n]
    (lazy-seq
      (cons (fizzbuzz targets n)
            (lazy-fizzbuzz targets (inc n)))))
  ([targets]
    (lazy-fizzbuzz targets 1)))

;;;;; EXAMPLES
(take 20 (lazy-fizzbuzz targets))
   => ("1" "2" "fizz" "4" "buzz" "fizz" "baz" "8" "fizz" "buzz" "11" "fizz" "13" "baz" "fizzbuzz" "16" "17" "fizz" "19" "buzz")

;; You can also specify an initial n
(take 20 (lazy-fizzbuzz targets 20))
   => ("buzz" "fizzbaz" "22" "23" "fizz" "buzz" "26" "fizz" "baz" "29" "fizzbuzz" "31" "32" "fizz" "34" "buzzbaz" "fizz" "37" "38" "fizz")

;; which is the same as...
(take 20 (drop 19 (lazy-fizzbuzz targets)))
   => ("buzz" "fizzbaz" "22" "23" "fizz" "buzz" "26" "fizz" "baz" "29" "fizzbuzz" "31" "32" "fizz" "34" "buzzbaz" "fizz" "37" "38" "fizz")

Context

StackExchange Code Review Q#13634, answer score: 5

Revisions (0)

No revisions yet.