Allow keys to be munged when encoding #34

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

Basically the opposite of #27.

- (str k#)))
+ (.writeFieldName ~jg (~key-fn (if (keyword? k#)
+ (.substring (str k#) 1)
+ (str k#))))
@jeremyheiler

jeremyheiler Jan 24, 2013

On second thought, maybe it should invoke key-fn or the existing code, not both.

I am bit curious as to why the build failed. It appears to be an issue with test.generative 1.4.0 not supporting Clojure 1.2 (the pom.xml for it says Clojure 1.3) but the master builds pass. What am I missing?

Owner

dakrone commented Jan 26, 2013

@jeremyheiler sorry, I don't mean to be ignoring this, I'm out of town on vacation, so I won't be able to look at it until Monday or Tuesday evening, just wanted to give you a heads up.

Owner

dakrone commented Jan 29, 2013

So I've been thinking about this for the last couple of days on vacation. I think something like this is out of scope for a JSON encoder. Changing the data before calling json/encode is something the user's application should handle.

In addition, adding this change makes all generation take about 3.75 times longer, which is definitely too much of an overhead.

As a workaround, something like this https://gist.github.com/485ad11b922598c0c77c could be used.

Thanks for the feedback and the workaround.

I guess it bothers me that I cannot make the following return true by providing a lower-case function.

(let [data "{\"foo\":\"bar\"}" upper-case (fn [k] (.toUpperCase (name k)))]
  (= data (json/encode (json/decode data upper-case))))

Do you have any ideas about the cause of the performance decrease?

Owner

dakrone commented Jan 30, 2013

The performance decrease in most likely because of the map destructuring in the generate method.

Also, this doesn't correctly work on sub-maps, so something like:

(json/encode {:foo "bar" :bar {:baz "quux"}} {:key-fn (fn [k] (.toUpperCase k))})

Renders:

{"FOO":"bar","BAR":{"baz":"quux"}}

instead of:

{"FOO":"bar","BAR":{"BAZ":"quux"}}

That's unfortunate. My goal was to not break backwards compatibility by making it optional.

If you feel strongly about key transformation being out of scope, then I might just take some time to create a library that does just that. If you don't mind, I would like to use your gist as a starting point. Otherwise, I would be happy to continue looking into allowing full key transformation in cheshire.

Owner

dakrone commented Feb 1, 2013

I think it would be better implemented as a library so that it would be widely used (I am quite sure there are people who would like to do key transformations outside of json encoding). That way it is easily used for people who would like it for their json encoding, but not all users incur the performance penalty of it.

So, I created this library. It works like this:

(transform-keys (comp keyword clojure.string/lower-case camel->dash)
                {"FooBar" [{"Fancy1" nil "R2D2" nil} {"MoreNo1se" nil}]})
;=> {:foo-bar [{:fancy1 nil, :r2-d2 nil} {:more-no1se nil}]}

It's still very alpha, so I would appreciate any feedback 😄

Repository: https://github.com/jeremyheiler/wharf

Owner

dakrone commented Mar 31, 2013

Neat, I'll check it out.

Owner

dakrone commented Apr 4, 2013

Closing this since it was addressed in #40 and released in 5.1.0.

@dakrone dakrone closed this Apr 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment