Clojurians Log v2
code-reviews 2020-07-02
@stopachka has joined the channel
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)
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
the common thing, if you don't need the return value, is to use try/catch with an error level log on failure
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?
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)
it might be clearer to have a special "failure" value for the promise
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)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
in fact- now that I think a few seconds longer, try just using keywordize-keys without the into call?
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
OK - then you'd need a recursive (into {} ...)
(yes indeed! found smth nice, on it!)
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)))love it! ended up going with something v close to above -> https://github.com/stopachka/jt/blob/master/server/jt/core.clj#L31-L48
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)
will look into adding smth similar, thank you!
that will work in many cases
mindblown will try! thank you!
https://github.com/stopachka/jt/pull/1/files#diff-0bbeb00ad8db9e2d48dfa64de4f775ecR280 instead of using future here you can provide :join false to run-jetty
great idea!
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
xD fair! will include in secrets