TheThingsArchive / java-app-sdk

The Things Network Application SDK for Java
MIT License
30 stars 25 forks source link

Replace JSONObject by java Objects and use Jackson for deserialization #37

Closed dennisg closed 7 years ago

dennisg commented 7 years ago

Are there any pressing reasons to include JSONObject in the exposed API ?

I would prefer receiving a raw data String over the use of a JSONObject with activation information.

This would allow me to use any other JSON parsing schemes available, now I'm forced to either:

cambierr commented 7 years ago

Well, the idea is to make user's life easier and returning an already-parsed object seems much more user-friendly than returning a String that the user need to parse later on.

Is there any particular reason you disagree with the choice of org.json ? It's a very first release so if you have a better json lib than this one, we could just move to it.

@FokkeZB any advice on this ?

dennisg commented 7 years ago

Hi Romain, thanks for the quick reply. much appreciated

Well, the thing is... 'already parsed' doesn't give me anything short of a Map, I would still have to investigate, parse it and base my implementation on it. Personally, I'm a fan of Gson, but others might like Jackson or something else. All would probably prefer the raw data; I'd rather leave it up to the integrator. It's just my opinion, not a big deal though.

cambierr commented 7 years ago

Since account already uses jackson, I could change the output data to a pure java object mapping the one from TTN. Do you think this would be better ? I think it's still better than raw data for the majority of users :)

FokkeZB commented 7 years ago

I'd say if you like it raw, you should just talk to the MQTT broker directly. It's an SDKs job to pre and post process to make live easier. I'm ok with using Jackson to return a true object, as that is also what you get from the Nodejs SDK.

Thanks for the input @dennisg!

dennisg commented 7 years ago

oh, it is more than welcome :-)

As said, it is not required to conform to my wishes.

True about the raw MQTT stream, but your client interface in Java takes away much of the cruft required to easily listen to events. For me converting it to a JSONObject adds nothing. Others might argue it does, opinions differ.

As an aside, the various places where an Object is returned it still is unclear to me whether it would always be safe to do a toString(), hope it would return JSON and parse that. adds to the confusion, to my opinion.

I would personally prefer either:

cambierr commented 7 years ago

I'll go with the first solution: I'll expand the Message I already return to be able to enclose both a string matching a single field, or an Object will all getters to match json

dennisg commented 7 years ago

👍 thanks!