Fork me on GitHub
#code-reviews
<
2019-09-04
>
jjttjj18:09:57

Yet another abstract style decision question from me: I'm wrapping an api. I want to offer users the ability to extend my wrapper The basic entrypoint is:

(req connection request)
I need the ability for users to extend my library so that they can: 1. Modify the request before it is sent (an example extension function would add missing default values to a request) 2. Modify the results of the request based on the request. (Examples would be taking a list of response messages and extracting only the relevant keyword from each message, or reducing them to a single value ) Would it make sense for req to be part of a protocol, with multiple other protocol functions handle-request and handle-result which are used internally? Would it be better to offer the ability for handlers functions (for both pre and post processing) to be passed to req? Or in general is it best to just require users to wrap this type of thing and do the pre-post processing in their own function?

noisesmith18:09:24

middleware is popular for this sort of thing

jjttjj18:09:51

yeah i've dabbled extensively with both middleware and interceptors... never could get things to feel quite right for reasons I can't quite articulate

jjttjj18:09:28

now that I mention it, middleware is probably the best of the options, yet for some reason I don't like it personally, I just end up getting tripped up by the pattern all the time (forgetting to call the next handler, reversing order of things, etc)

noisesmith18:09:35

my experience was to use middleware well and reliably write new middleware I had to spend a few hours just writing weird middlewares and experimenting in the repl

noisesmith19:09:11

it's an idiom that you need to learn if you want to use it properly IMHO

jjttjj19:09:50

gotcha, thanks for the input 👍

noisesmith19:09:22

I recently worked on a project doing exactly what you describe - we needed a way to extend and vary api consuming code

noisesmith19:09:53

it started as copy/paste first of course, then I tried a strategy-pattern style protocol version

noisesmith19:09:04

what ended up sticking and being reliable was middleware

jjttjj20:09:18

Back to:

(req connection request)
If you need to access the connection inside the middleware, would you put the connection inside the request map? or have you internal middleware functions all take 2 arguments (the connection and the request)?

noisesmith20:09:20

the most common thing is definitely to wrap everything in one map (ring style) when using middleware, but two args also works - all the middleware would just have to agree on the convention

jjttjj20:09:38

I feel like in my previous experiments the "put things into one big map" lead to sloppyness in just using that map as a way to hold everything, leading to complex dependencies between handlers. The 2 args seems like it might do. Thanks!

👍 4
noisesmith20:09:57

spec might also help - eg. each middleware defining a spec for the data it uses and adds

jjttjj20:09:20

good point

noisesmith20:09:20

and yeah, agreed that when used in an undisciplined way the "ad-hoc map of everything" argument leads to messy and brittle results

dominicm09:09:39

Curious: have you tried anything to combat this.

dominicm09:09:51

I'm currently undergoing investigation into alternatives

noisesmith16:09:04

I have some suspicions / guesses about when and why it happens (and what went right in places where it doesn't happen), but nothing really formal atm

dominicm16:09:40

That's perfect

dominicm16:09:41

:) we are still rooting the domain out, we feel stuck between a rock and a hard place. The only ideas we have we have seen lead to this same end place.

noisesmith16:09:25

my knee jerk response is that an undisciplined mix of random parameters is a sign that the shape of the current design doesn't intuitively match up with the domain, and the map keys are being forced to act as an adaptor between the actual data flow and the one the domain needs

noisesmith16:09:40

but that's just a hunch, not having seen your code

jjttjj16:09:14

I think putting everything in a map that get's passed everywhere starts to become similar to mutable state. It actually kind of is mutable state, you're just kind of using the state held by the environment (ie stack frames) instead of being explicit about it (ie using atoms

noisesmith16:09:27

except you have the guarantee that nothing "mutates" from any call unless you access it from the return value

noisesmith16:09:49

but in a big picture it does introduce the same sorts of problems

dominicm20:09:49

The anti pattern I'm particularly struggling with is the introduction of a big bag of state made up of 90% of your system. Then somewhere along the pipeline that map gets enhanced. And in different parts of the codebase it has different keys but the same name.

noisesmith20:09:06

oh yeah... I remember that problem from a ball of mud product I worked on a few years back

jjttjj20:09:12

funny, I was just thinking about how this discussion ties into the use of a system map (ie the map of statefull things used in component and other state management libs). Like I think that concept is slightly different in some ways than that "map which passes through a pipeline" but in other ways it's similar

noisesmith20:09:05

the system map version avoids @U09LZR36F’s issue where the same function gets different variants of the map based on the code path that led to the call

noisesmith20:09:24

at least the system map has a single point of generation / truth

jjttjj20:09:46

good point

dominicm20:09:02

That depends how granular you go. Even with 2 entry points (Web server, event handler) taking different dependency sets and calling them "deps" it's very confusing. Especially as various parts add derivations to that map (db snapshots, removing the record wrapper) the actual function that performs behaviour may not be taking something that looks anything like your source of truth. You're left baffled for hours wondering how it's calling mongodb directly like that.

dominicm20:09:30

If you expand that to each endpoint in your system (an attempt at avoiding the massive deps map and attempting to make each endpoint easier to follow by minimizing dependencies) you can end up with maps which are passed around with injection into them. Having a map of dependencies is synonymous with recipe for disaster in my mind right now.

dominicm20:09:35

I've been considering making a non-assocable map to prevent it happening 😂 I don't want to go that far. I'd rather find a new abstraction.

noisesmith20:09:12

dynamically bound symbols instead of map keys? lol

dominicm04:09:16

We joke, but maybe that's not a terrible idea?

dominicm04:09:32

My only hang up is async code

noisesmith18:09:26

async (go blocks and futures and async/thread) all propagate dynamic bindings

noisesmith18:09:53

now this would be the binding from their call chain and wouldn't reflect later changes under other call chains...

dominicm20:09:58

NIO is way more important in http, which means something like netty, which I don't think does dynamic bindings.

noisesmith20:09:51

you could pass it a function that is created via binding-conveyor-fn

(ins)user=> (source future)
(defmacro future
  "Takes a body of expressions and yields a future object that will
  invoke the body in another thread, and will cache the result and
  return it on all subsequent calls to deref/@. If the computation has
  not yet finished, calls to deref/@ will block, unless the variant of
  deref with timeout is used. See also - realized?."
  {:added "1.1"}
  [& body] `(future-call (^{:once true} fn* [] ~@body)))
nil
(ins)user=> (source future-call)
(defn future-call 
  "Takes a function of no args and yields a future object that will
  invoke the function in another thread, and will cache the result and
  return it on all subsequent calls to deref/@. If the computation has
  not yet finished, calls to deref/@ will block, unless the variant
  of deref with timeout is used. See also - realized?."
  {:added "1.1"
   :static true}
  [f]
  (let [f (binding-conveyor-fn f)
        fut (.submit clojure.lang.Agent/soloExecutor ^Callable f)]
    (reify 
     clojure.lang.IDeref 
     (deref [_] (deref-future fut))
     clojure.lang.IBlockingDeref
     (deref
      [_ timeout-ms timeout-val]
      (deref-future fut timeout-ms timeout-val))
     clojure.lang.IPending
     (isRealized [_] (.isDone fut))
     java.util.concurrent.Future
      (get [_] (.get fut))
      (get [_ timeout unit] (.get fut timeout unit))
      (isCancelled [_] (.isCancelled fut))
      (isDone [_] (.isDone fut))
      (cancel [_ interrupt?] (.cancel fut interrupt?)))))
nil

dominicm06:09:39

Then you have to remember to do that :p

noisesmith20:09:09

one of the main things I miss about compile time type checking is the compiler telling me where my refactor is incomplete, hiding a gumbo of objects in a map makes this problem even worse

jjttjj20:09:59

Yes, I've definitely been wrestling with this type of thing a lot lately

robertfw21:09:53

I've been getting some pushback over the use of or to choose between values - e.g., picking one or another value that might be in a map, like (or (:hopefully-is-present my-map) (:fallback-if-not my-map)). The complaint being that or should be used for strictly logical/boolean operations. Does anyone have opinions on whether that use is idiomatic, and if not, what should be used instead?

noisesmith22:09:05

or is idiomatic, but also keywords take a second arg that acts like or

noisesmith22:09:28

so in this case, just do (:hopefully-is-present my-map (:fallback-if-not my-map))

noisesmith22:09:16

but really, or isn't boolean or logical in lisps

robertfw22:09:43

yeah that's a poor example in that case - it's used elsewhere in other ways though. thanks!

robertfw22:09:00

I'm currently the sole developer on this clojure project - we have a legacy clojure project from some years back but as is, I'm the only full time clojure dev. I get reviews from our architect who has some clojure experience but otherwise I'm left as the sole defender of some of these practices, so it's been useful having this community. thanks!

robertfw22:09:20

(this is my first big clojure project after some very basic personal dabbling)

jjttjj22:09:46

I personally don't mind or used like this generally either

8
noisesmith22:09:14

literally the only objections I've seen are from people not familiar with lisps

8
noisesmith22:09:22

it's a standard lisp idiom

robertfw23:09:54

I will head into my next code review armed with the phrase "standard lisp idiom" 😉 Thanks folks

noisesmith23:09:24

another idiom for "first key present"

user=> (some {:b 2 :c 3} [:a :b :c])
2

💯 8
noisesmith23:09:34

since maps acts as functions over keys

robertfw23:09:40

oh neat. I haven't seen that one before

jjttjj20:09:12

funny, I was just thinking about how this discussion ties into the use of a system map (ie the map of statefull things used in component and other state management libs). Like I think that concept is slightly different in some ways than that "map which passes through a pipeline" but in other ways it's similar