-
Notifications
You must be signed in to change notification settings - Fork 48
Description
Hi @weavejester!
This Ring middleware lib has a good design — it's succinct (minimalistic codebase and API) and does one thing (read/write JSON from/to req/resp) and does it well (in terms of performance).
Still, it strongly depends on a particular JSON mapping implementation library, a Cheshire in this case. And while it is apparently the most popular library for this purpose, there are numerous projects out there that use its alternatives, be it another Jackson-based metosin/jsonista or org.clojure/data.json with zero external dependencies. And, I hope you agree, it makes sense to (re)use the same tool for the same tasks within the same project (possibly, with the same set of configs).
I understand what could have caused such a choice before (historical reasons), but wouldn't it be better to leave this choice to the user themselves nowadays? This can be achieved by abstracting from the specific implementation and dynamically linking any supported library from the current classpath (much alike the clj-http.client does it when looks for Cheshire). We've achieved a similar goal in the Telegram Bot API client, so there's already a reference code anyone could review.
But for the ring-json, the task is a bit trickier due to the use of streams API and passing the mapping parameters along the way. And I'd propose to: a) split different mapper-specific implementations into different namespaces/files, if this is at all possible, to keep the codebase clean, and b) test the performance degradation with JMH and, if it is substantial, leave this library as it is (backed by Cheshire), perhaps providing alternatives (backed by the other two mappers) as separate artifacts.
Like to hear your thoughts!
Cheers,
Mark