Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More concise use of parameterized liberator resources? #95

Closed
jonathanj opened this issue Nov 5, 2015 · 4 comments
Closed

More concise use of parameterized liberator resources? #95

jonathanj opened this issue Nov 5, 2015 · 4 comments

Comments

@jonathanj
Copy link

I have some code:

(defresource foo [id]
  :available-media-types ["application/json"]
  :allowed-methods [:get]
  :handle-ok
  (fn [ctx]
    {:hello id}))

(def routes ["/foo/" {[:id] foo}])

(def app (-> (make-handler routes) (wrap-default api-default)))

This doesn't work as-is because something is trying to cast the foo function to an associative structure, so this seemed like it might work:

(def routes ["/foo/" {[:id] (fn [req] (foo (get-in req [:route-params :id])}])

But (foo x) produces a handler function that closes over x, it seems since once again something was trying to cast a function to an associative structure.

What I ended up with was:

(def routes ["/foo/" {[:id] (fn [req] ((foo (get-in req [:route-params :id]) req)}])

Which is kind of messy, especially when compared to something like Compojure:

(defroutes routes (ANY "/foo/:id" [id] (foo id)))

Am I missing something?

@malcolmsparks
Copy link
Contributor

Thanks for this. I don't think you are missing anything.

Compojure uses a macro designed for destructuring route params, so is much better optimized for this example. I've toyed with some destructuring macro layered upon bidi but haven't come up with anything worthwhile yet.

@jonathanj
Copy link
Author

After filing this issue I experimented with some different approaches. My previous example just passed the :id route parameter to the resource but obviously that's not very useful alone, presume it wanted to look up the identifier somehow to produce a response:

(defresource foo [lookup-identifier]
  :exists?
  (fn [ctx] {::id (get-in ctx [:request :route-params :id])}
  :handle-ok
  (fn [ctx] (lookup-identifier (::id ctx))))

(def routes ["/foo/" {[:id] (foo db/lookup-identifier)}])

I'm quite happy with this since it reduces the complexity in my routes dramatically and factors my resources into a form that is better abstracted and so easier to test. Indeed, it would be nice to have the route parameters decoupled from the resource even further but even then I think writing some domain-specific convenience functions might give good results for my purposes.

Thanks for taking a look though, I definitely wouldn't object to some convenience improvements to this aspect.

@zendevil
Copy link

@jonathanj, would you explain why this way "factors my resources into a form that is better abstracted and so easier to test"? i.e., why is it better than doing (def routes ["/foo/" {[:id] (foo nil)}]), and then

(defresource foo [_]
  :exists?
  (fn [ctx] {::id (get-in ctx [:request :route-params :id])}
  :handle-ok
  (fn [ctx] (db/lookup-identifier (::id ctx))))

@jonathanj
Copy link
Author

(Disclaimer: This was posted 5 years ago, and I’m not sure I recall the specifics in great detail.)

If I had to guess at answering your question, it’s because the “how to look up an identifier” is parameterised, instead of being baked into the resource as in your example.

Which is easier to test and more reusable because the behaviour is supplied externally and can be varied as required by different callers, for differing data queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants