3
\$\begingroup\$

I'm doing the exercices from clojureacademy. Here are the instructions for this one.

Bob is a lackadaisical teenager. In conversation, his responses are very limited.

  • Returns "Whatever." if given phrase is one of the following inputs:
    • "Tom-ay-to, tom-aaaah-to."
    • "Let's go make out behind the gym!"
    • "It's OK if you don't want to go to the DMV."
    • "Ending with ? means a question."
    • "1, 2, 3"
  • Returns "Woah, chill out!" if given phrase is one of the following inputs:

    • "WATCH OUT!"
    • "WHAT THE HELL WERE YOU THINKING?"
    • "ZOMG THE %^@#$(^ ZOMBIES ARE COMING!!11!!1!"
    • "1, 2, 3 GO!"
    • "I HATE YOU"
  • Returns "Sure." if given phrase is one of the following inputs:

    • "Does this cryogenic chamber make me look fat?"
    • "4?"
  • Returns "Fine. Be that way!" if given phrase is one of the following inputs:
    • ""
    • " "

Here is my code in src/ex1_bob/core.clj:

(ns ex1-bob.core
  (:gen-class))

; responses
; =======================
(def whatever "Whatever.")
(def chill-out "Woah, chill out!")
(def sure "Sure.")
(def fine "Fine. Be that way!")

; triggers
; =======================
(def sure-triggers
  ["Does this cryogenic chamber make me look fat?" "4?"])
(def whatever-triggers
  ["Tom-ay-to, tom-aaaah-to."
   "Let's go make out behind the gym!"
   "It's OK if you don't want to go to the DMV."
   "Ending with ? means a question."
   "1, 2, 3"])
(def chill-out-triggers
  ["WATCH OUT!"
   "WHAT THE HELL WERE YOU THINKING?"
   "ZOMG THE %^*@#$(*^ ZOMBIES ARE COMING!!11!!1!"
   "1, 2, 3 GO!"
   "I HATE YOU"])



(defn has-phrase
  "return `true` if the given phrase is found in the collection, `false` otherwise"
  [phrase coll] 
  (if (some #(= phrase %) coll)
    true
    false))

(defn response-for
  "return `true` if the given phrase is found in the collection, `false` otherwise"
  [phrase]
    (cond 
      (has-phrase phrase whatever-triggers)  whatever
      (has-phrase phrase chill-out-triggers) chill-out
      (has-phrase phrase sure-triggers)      sure
      (= (clojure.string/trim phrase) "")    fine))

I also wrote some tests in test/ex1_bob/core_test.clj:

(ns ex1-bob.core-test
  (:require [clojure.test :refer :all]
            [ex1-bob.core :refer :all]))

; This does not work because `is` is a macro, and it seems it does not like to
; be used in a `map` or with `comp`.
;
; (deftest non-working-response-for-chill-out-triggers
;   (doall (map (comp is #(= % chill-out) response-for) chill-out-triggers)))

(deftest response-for-chill-out-triggers
  (is (every? #(= % chill-out) (map response-for chill-out-triggers))))

(deftest response-for-whatever-triggers
  (is (every? #(= % whatever) (map response-for whatever-triggers))))

(deftest response-for-sure-triggers
  (is (every? #(= % sure) (map response-for sure-triggers))))

(deftest response-for-empty-string
  (is (every? #(= % fine) (map response-for [""]))))

Two things make me a bit unhappy about this code:

  • the has-phrase function seems a bit silly. Do I really have to write a function to return true/false if an element is found in a collection? I feel that there must be a function that does this already
  • I first wrote my unit test with:

    (deftest non-working-response-for-chill-out-triggers
      (doall (map (comp is #(= % chill-out) response-for) chill-out-triggers)))
    

    The idea is to have one assertion for each item, which I assumed would give me a better error message in case of a failure (because I know which item failed). But it seems that is is a macro and somehow, the interpreter doesn't like this.

\$\endgroup\$
2
\$\begingroup\$

First, working top-down, your triggers at the top should be in sets (#{}), not vectors (you'll see why a little farther down):

(def sure-triggers
  #{"Does this cryogenic chamber make me look fat?"
    "4?"})

(def whatever-triggers
  #{"Tom-ay-to, tom-aaaah-to."
    "Let's go make out behind the gym!"
    "It's OK if you don't want to go to the DMV."
    "Ending with ? means a question."
    "1, 2, 3"})

(def chill-out-triggers
  #{"WATCH OUT!"
    "WHAT THE HELL WERE YOU THINKING?"
    "ZOMG THE %^*@#$(*^ ZOMBIES ARE COMING!!11!!1!"
    "1, 2, 3 GO!"
    "I HATE YOU"})

has-phrase has a couple things that can be improved:

  • Clojure ends predicates with a ? by convention. I'd rename it to has-phrase?.

  • some already returns a truthy/falsey value. (if pred? true false) is unnecessary.

Just reduce it to:

(defn has-phrase?
  "Return whether or not the given phrase is found in the collection"
  [phrase coll]
  (some #(= phrase %) coll))

BUT, now that you're using sets instead of vectors, this whole function is unnecessary:

(whatever-triggers "1, 2, 3")
=> "1, 2, 3" ; Truthy

(whatever-triggers "Not in the set")
=> nil

Sets themselves can be used as functions. If the item is in the set, they return the item back (a truthy result), else they return nil (falsey). This will also be much faster, since now you don't need to have some iterate potentially the entire vector (although obviously speed isn't a factor here). This changes reponse-for to:

(defn response-for
  "return `true` if the given phrase is found in the collection, `false` otherwise"
  [phrase]
  (cond
    (whatever-triggers phrase) whatever
    (chill-out-triggers phrase) chill-out
    (sure-triggers phrase) sure
    (= (clojure.string/trim phrase) "") fine))

This could be changed using some some trickery. some returns the first truthy value returned by its predicate. If you pass it pairs of [trigger-set response], you can iterate over the the pairs to find the first one that satisfies it, then return the response:

(defn response-for2 [phrase]
  (some (fn [[trigger-set response]]
          (when (trigger-set phrase)
            response))

        [[sure-triggers sure]
         [chill-out-triggers chill-out]
         [whatever-triggers whatever]
         [#{"" " "} fine]]))

Now, in this particular example, I wouldn't recommend doing this. It's not as clear as what you already had. I have used (some #(when ...)) before though, and it works great in some cases. I just thought I'd show you it here in case you run into a similar problem where this can be applied.

I'm sure something could be done using a map mapping triggers to responses, but I just did another Clojure review, and my brain's tapped out.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the thorough review, I learnt a lot! \$\endgroup\$ – little-dude Mar 5 at 13:00
1
\$\begingroup\$

For some reason I can't open the link to the question site. But "Returns [X] if given phrase is one of the following inputs", to me, does not mean it only returns that for that inputs. It also doesn't say you should return nil otherwise.

If we take the listed inputs to be test cases; then baking them in the implementation is not good.

I understand the cases to be as such: - blank (nothing but whitespace) - question (ends with \?, as hinted in example) - shouting (all letters are uppercase) - anyhing else

\$\endgroup\$
  • \$\begingroup\$ Oh I had not thought about that. Now that you say it, it seems kind of obvious that this is probably the right thing to do... \$\endgroup\$ – little-dude Mar 5 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.