search

mixpanel / mixpanel-java

Other
49 stars 37 forks source link

Check null on JSONObject.getNames #10

Closed ceedubs closed 10 years ago

ceedubs commented 10 years ago

JSONObject.getNames has the awful property that if the argument is a JSONObject with no keys, it returns null instead of an empty array. When conditional logic is used to generate modifiers, it's easy to end up getting one without any keys.

joeatwork commented 10 years ago

This is a nice fix! Thanks so much for your contribution!

I hate to ask for dessert when given a free dinner, but would you consider adding a test case to your Pull Request?

ceedubs commented 10 years ago

If I'm honest I will probably never get around to adding tests to this PR. Java makes me unhappy. Sorry!

joeatwork commented 10 years ago

Fair enough- I'll write a test for this before I merge it.

(Out of curiosity, are you using the Java lib from Scala or Clojure or something non-Java? Because that's awesome :)

ceedubs commented 10 years ago

@joeatwork yeah, I'm using it from Scala. If there were a Scala version that performed async IO for the service calls I'd happily switch to that and would probably be more inclined to submit PRs with unit tests ;)