@seancorfield Thanks for the link, pretty interesting (even though I don’t know that I fully understand it!).
For logging, I’d be inclined to leave it in the business logic code even if it does technically make those functions impure, since logging doesn’t affect the application behavior from the user’s standpoint. Though it also might be convenient enough to have business logic functions just include a :biff.chain/queue [:my-logger ...], :my-logger/stuff-to-log [:info "abc123"] in their results…
For unexpected exceptions I’d let them bubble up into the ring middleware as usual; for expected exceptions I’d lean towards having the effectful code return data instead and having the subsequent business logic check for failure and return early if needed (by returning an empty :biff.chain/queue). e.g. in my example code, to begin with I probably should have set :biff.chain.http/input {:throw-exceptions false, ...} and then have the ::add-urls clause check the response status:
::add-urls
(let [{:keys [status] :as output} (:biff.chain.http/output ctx)
feed-urls (when (<= 200 status 299)
(->> (lib.rss/parse-urls output)
(mapv :url)
(take 20)
vec))]
(if (empty? feed-urls)
{:status 303
:biff.response/route-name :app.subscriptions.add/page
:biff.response/route-params {:error "invalid-rss-feed"}}
...))
The more paths through your business logic, the more passes through the same handler code and the more branches you would have in case.
I am wary of that… though it also might end up being ok. Most of our handlers at work don’t have that many effects. The most complex one that comes to mind has maybe 6 (a couple redis calls, a couple s3 calls, a couple calls to an internal service…) and most of the logic is in a single path, with the other paths being early returns. So for that handler at least I think the case form would end up pretty readable; each clause would just be “if this is true, return now, else go to the next clause”, similar to the ::add-urls clause. There may be some handlers that end up being pretty hairy, but if most of them are ok, then maybe it’s ok.
(This is a ~4-year-old, 80k-line, mostly-monolithic codebase–I’m sure there are other codebases with more effects)
However! Last night I also started musing about if some macro magic could take some imperative code and turn it into the pure case style. I’m imagining something that looks kind of like core.async–here’s the original imperative function with a hypothetical purify macro, and with the effectful bits wrapped in (effect ...):
(fn [{:keys [session
params]
:as ctx}]
(purify
(let [url (lib.rss/fix-url (:url params))
http-response (effect (http/get url {"User-Agent" "https://yakread.com"}))
feed-urls (->> (lib.rss/parse-urls (assoc http-response :url url))
(mapv :url)
(take 20)
vec)
tx (for [url feed-urls]
{:db/doc-type :conn/rss
:db.op/upsert {:conn/user (:uid session)
:conn.rss/url url}
:conn.rss/subscribed-at :db/now})]
(if (empty? feed-urls)
{:status 303
:biff.response/route-name :app.subscriptions.add/page
:biff.response/route-params {:error "invalid-rss-feed"}}
(do
(effect (biff/submit-tx ctx tx))
{:status 303
:biff.response/route-name :app.subscriptions/page
:biff.response/route-params {:added-feeds (count feed-urls)}})))))
I haven’t thought about that deeply at all; there might be some obvious reason why that wouldn’t work at all. But perhaps another thing to experiment with.
@maxweber that approach looks conceptually pretty similar to what I’m proposing–basically instead of using a single function with different “states” / case clauses, you’re breaking it out into separate functions. The other big difference though is where the conditional logic goes. One thing I like about my approach is that the conditional logic gets pushed down into the pure business logic functions, so I don’t need to write a custom parent function at all (which would have effects, and which would need to be tested, which is the situation I’m trying to avoid), I just use orchestrate.
e.g. say you had a handler that made a request to service A, then based on the result it makes another request to either service B or service C (and then do more computation on the responses)? That conditional logic would need to live in handle!, wouldn’t it?
On the other hand: even if you don’t break everything down into world functions, it does sound like a potentially happy medium to not worry about separating co-effecting code (using re-frame’s terminology) from the business logic and just try to push the side effects to the end + return them as data–e.g. something like this:
(fn [{:keys [session
params
http-client]
:as ctx}]
(let [url (lib.rss/fix-url (:url params))
http-response (http-client url {"User-Agent" "https://yakread.com"})
feed-urls (->> (lib.rss/parse-urls (assoc http-response :url url))
(mapv :url)
(take 20)
vec)]
(if (empty? feed-urls)
{:status 303
:biff.response/route-name :app.subscriptions.add/page
:biff.response/route-params {:error "invalid-rss-feed"}}
{:status 303
:biff.response/route-name :app.subscriptions/page
:biff.response/route-params {:added-feeds (count feed-urls)}
:biff.side-effect/tx (for [url feed-urls]
{:db/doc-type :conn/rss
:db.op/upsert {:conn/user (:uid session)
:conn.rss/url url}
:conn.rss/subscribed-at :db/now})})))
(off topic: just noticed I evidently have a jacob and a jacobobryant account on this site, and managed to write my two posts here from different accounts somehow…)