Fork me on GitHub
#code-reviews
<
2019-09-21
>
Daw-Ran Liou00:09:11

Hi folks, I created the library: https://github.com/dawran6/emoji. Currently the emoji.core ns is about 100 loc. Feel free to drop your comments here or on the Github issues. Thank you! 😄

Vincent Cantin05:09:48

I took a look at the code and it seems that you could get better performances using transducers.

👍 4
dominicm05:09:58

I don't think that would make a significant difference? There's only 2 levels of lazy sequence.

Daw-Ran Liou14:09:01

It might be a great chance for me to learn about transducers. I'll give it a try. Thanks @U8MJBRSR5 , @U09LZR36F

Vincent Cantin15:09:14

@U09LZR36F transducers remove all the cost of the laziness and also remove intermediate buffering as well. For a library, it is often desirable.

Daw-Ran Liou15:09:27

Appreciate! I came across your blog post just a while ago. Now is a good time to practice it 🙂

😄 4
Vincent Cantin15:09:36

Feel free to ask in #code-reviews if you have any question on transducers, remember to tag me.

parrot 4
Vincent Cantin15:09:32

or tag me in the issue if needed (@green-coder)

dharrigan11:09:28

What would be a better way of doing this?

dharrigan11:09:43

(defn filter-trip
  [trip]
  (cond
   (= nil (:id trip)) false
   (= nil (:tripData trip)) false
   (= nil (:tenantId trip)) false
   (= nil (-> trip :tripData :startLocation)) false
   (= nil (-> trip :tripData :endLocation)) false
   (= nil (-> trip :tripData :segments)) false
   (= nil (:device trip)) false
   (= nil (-> trip :device :id)) false
   ((contains? #{"BAD" "UNUSABLE"} (:accuracy trip))) false
   :else trip))

dharrigan11:09:26

I feel it's a bit clunky

jaihindhreddy15:09:18

@dharrigan Maybe this?

(defn legit-trip?
  [{{:keys [startLocation endLocation segments]} :tripData
    {:keys [id]} :device
    :as trip}]
  (and startLocation endLocation segments id
       (every? trip [:tenantId :id])
       (not (#{"BAD" "UNUSABLE"} (:accuracy trip)))))

jaihindhreddy15:09:37

Your fn seems to be a predicate

jaihindhreddy15:09:13

But if you want the trip to be returned in the truthy case (for use with keep maybe), then you can wrap the and expr in an if and return it.

dharrigan15:09:30

nice, thank you - definitely looks sweeter 🙂

jaihindhreddy15:09:34

Both these versions return truthy even if :accuracy isn't present in trip. Is that intentional?

jaihindhreddy15:09:51

@dharrigan An alternative version, if the prev. one has too much destructuring for your taste 😄

(defn legit-trip?
  [trip]
  (and (not (some #(nil? (get-in trip %))
               [[:id]
                [:tenantId]
                [:device :id]
                [:tripData :startLocation]
                [:tripData :endLocation]
                [:tripData :segments]]))
       (not (#{"BAD" "UNUSABLE"} (:accuracy trip)))))

dharrigan15:09:43

ah, that was an oversight, if accuracy is not present, it should be false

dharrigan15:09:22

I always um and ah about destructuring in function arguments - sometimes I like, sometimes...not so sure 🙂

jaihindhreddy15:09:06

@dharrigan A little destructuring is probably best. Here's one with "a little" destructuring

(defn legit-trip?
  [{:keys [tripData device] :as trip}]
  (not (or (some #(nil? (% trip)) [:id :accuracy :tenantId])
           (some #(nil? (% tripData)) [:startLocation :endLocation :segments])
           (nil? (:id device))
           (#{"BAD" "UNUSABLE"} (:accuracy trip)))))

jaihindhreddy15:09:25

I'll stop tagging you now. Sorry if noise 😅

dharrigan15:09:58

It's okay - really appreciate helpful feedback, I like to learn 🙂

💯 4
dharrigan15:09:02

Thanks and have a nice day!

👍 8