code-reviews

2020-07-02T18:37:42.105Z

Hey team, created a quick clojure service "journaltogether" • idea is: service sends an email to a few friends, asking how their day was • they all answer • the next day, we receive a summary Haven't used clojure in a while -- would love a clojurian's take on the code. Esp on uses of futures, abstractions, etc https://github.com/stopachka/jt/pull/1/files (~about 300 loc)

2020-07-02T18:49:59.106200Z

any exceptions or errors inside the future calls will be caught by the future, and only get rethrown if you attempt to access the return value of the future

👍 1
2020-07-02T18:50:16.106800Z

the common thing, if you don't need the return value, is to use try/catch with an error level log on failure

👍 1
2020-07-02T18:50:45.107500Z

if the return value is in any way useful, you can ensure you see the exception by ensuring that deref occurs if it is realized?

2020-07-02T18:51:28.108200Z

gotcha! so to make sure something like this: https://github.com/stopachka/jt/pull/1/files#diff-0bbeb00ad8db9e2d48dfa64de4f775ecR104 is a fine way to "reject" a promse • (i can just throw the error, if someone who is using it derefs, they will get it)

2020-07-02T18:53:56.108700Z

it might be clearer to have a special "failure" value for the promise

2020-07-02T18:55:04.110100Z

also here: https://github.com/stopachka/jt/pull/1/files#diff-0bbeb00ad8db9e2d48dfa64de4f775ecR101-R102 just a small style thing, into takes an optional transducer arg that can keywordize a key, you don't need to do it in two steps:

(into {} (map (fn [[k v]] [(keyword k) v])) src)

2020-07-02T18:55:58.111Z

of course you'd want that fn to actually be something like (defn keywordize-kv ...) but the above version works in a repl for demonstration

2020-07-02T18:56:31.111500Z

in fact- now that I think a few seconds longer, try just using keywordize-keys without the into call?

2020-07-02T18:56:43.111700Z

that will work in many cases

2020-07-02T18:56:57.112Z

:mindblown: will try! thank you!

2020-07-02T18:58:06.112400Z

https://github.com/stopachka/jt/pull/1/files#diff-0bbeb00ad8db9e2d48dfa64de4f775ecR280 instead of using future here you can provide :join false to run-jetty

2020-07-02T18:59:46.113100Z

your friends might not appreciate this being plain text in a public repo - you could put it in a resource file outside version control with configurable location https://github.com/stopachka/jt/pull/1/files#diff-0bbeb00ad8db9e2d48dfa64de4f775ecR148

2020-07-02T19:00:16.113300Z

xD fair! will include in secrets

2020-07-02T19:00:28.113500Z

great idea!

2020-07-02T19:00:38.113700Z

works!

2020-07-02T19:01:14.113900Z

one thing i realized though: if i get a nested map from firebase, keywordize-keys won't work on it (I think this is because firebase returns something that isn't considered by keywordize-keys to be a map. will play around

2020-07-02T19:04:11.114100Z

OK - then you'd need a recursive (into {} ...)

2020-07-02T19:16:36.114300Z

(yes indeed! found smth nice, on it!)

2020-07-02T19:25:48.114500Z

oh - I thought maybe it would be easy, came up with this:

(import (java.util Collection
                   Map))

(defn deep-into-map
  [transform node]
  (let [prepare (map (comp transform (partial deep-into-map transform)))]
    (cond (isa? (class node) Map)
          (into {} prepare node)
          (and (isa? (class node) Collection)
               (every? #(and (isa? (class %) Collection)
                             (= (count %) 2))
                       node))
          (into {}
                (comp (map vec) prepare)
                node)
          :else
          node)))

❤️ 1
2020-07-02T20:17:01.114800Z

love it! ended up going with something v close to above -> https://github.com/stopachka/jt/blob/master/server/jt/core.clj#L31-L48

2020-07-02T20:40:39.115100Z

the big gotcha with your version is that java allows maps with non-string keys, that's why I made the transformation object a function (that cold be specialized for your known input)

👍 1
2020-07-02T20:46:54.115400Z

I see!

2020-07-02T20:48:30.115600Z

will look into adding smth similar, thank you!