Fork me on GitHub
#braveandtrue
<
2018-12-14
>
manutter5112:12:22

@anantpaatra Yeah, it looks to me like the function description is somewhat ambiguous, for the reasons you mention. Your code looks mostly good, in that it shows you’ve understood what the lesson is trying to teach. In your first answer, there’s a subtle bug in the append, which you will see if you pass in a suspect that fails validation. Also, you probably want if there rather than cond, since you’re only checking one predicate clause.

manutter5112:12:22

Your valid? function looks fine, but if I might make a stylistic suggestion: I would probably write it like this:

(defn valid?
  [validator record]
  (let [name-valid? (:name validator)
        glitter-index-valid? (:glitter-index validator)]
    (and (name-valid? (:name record))
         (glitter-index-valid? (:glitter-index record)))))

manutter5112:12:53

This is a style I think of as “executable documentation.” By using let to give expressive names to the validation functions, it becomes easier to understand what the code is doing just by reading the code. Compare that to ((:name validator) (:name record))--it’s perfectly legitimate Clojure, and experienced Clojure devs can probably see right away what it’s doing. Maybe. For me, it takes a little bit of extra effort to parse out that (:name validator) must be pulling a validation function out of the validator map because (:name validator) is the first item in the list.

manutter5112:12:56

Probably just a matter of personal experience and preference, but when I go back and look at my old code, I find the let style much easier to read, and thus much faster.

manutter5112:12:12

Also, in Clojure, the core functions like assoc and conj and so on have a convention that the collection comes before the thing you’re adding, so it might be a good idea to write your append function to take [list-of-suspects new-suspect]

manutter5112:12:08

Your second approach looks like it’s working as intended (and that’s a clever use of and to get it to return the unchanged record if everything else passes 🙂), but you probably don’t want to do it that way, since that could end up giving you a list of suspects that nothing but falses.

anantpaatra21:12:47

I completely agree with everything you said and learned from it. Using let like that makes complete sense and the end result is elegant, I'll try to incorporate that from now on.

anantpaatra21:12:10

The convention on the order of arguments also makes total sense and I even felt something weird with that function but couldn't see what it was.

anantpaatra21:12:14

So you don't think that there is a "clojurian" way to interpret what that return value should be (since it is ambiguous)?

anantpaatra21:12:06

I read "idiomatic" everywhere in the Clojure world so maybe I just try too hard to understand what people mean by it. But the book is quite interesting now and things are flowing, I'm way in the fifth chapter and I guess from now on the meaning of that will just become clearer.

manutter5121:12:21

Yeah, “idiomatic” mostly means “people who have been doing this for a while know the best tricks” 🙂

manutter5121:12:01

But in this case I think the way the exercise was defined was perhaps a bit unclear.

anantpaatra21:12:40

Thank you again for your time haha Clojurian channels can be very quiet (this intimidated me for a long time) so I hope I'm asking relevant questions.

manutter5121:12:12

Sure, any questions you’d like an answer for are relevant to this channel

manutter5121:12:42

Well, I mean apart from “How can I make a million dollars?” or “What’s your credit card number?”