Open Geeber opened 9 years ago
I think it'd be good to call out a few popular Map builders in the docs, e.g. google commons ImmutableMap
. That way people are aware of this type of tool, but we don't have to pick one, declare it as a hard dependency, wrap it, etc...
I've created some utility methods for converting "flat" maps to nested maps for use by the KeenClient (included in part of my current project, see here: https://github.com/CourseScheduler/scheduler-legacy/blob/master/src/main/java/io/devyse/scheduler/analytics/keen/KeenUtils.java).
I can submit this as an addition to KeenUtils or wherever is appropriate if you have interest.
Very cool!
I think this would make sense to include directly in the SDK only if a) flat map conversion is common to many SDK users and b) good documentation of how to use a map builder would still not address this need. @mikereinhold @Geeber what do you think? Should this capability be brought into the SDK, or is it something we talk about or link to from the docs?
My personal perspective is that, since Keen expects a nested map, it would make sense to include some form of conversion method or class that provides an automatically nested map.
That being said, if you do not want to bring the functionality into SDK directly, I can break the code out separate from the rest of my application so that it can be linked to and used by anyone.
What would you guys think about creating a separate event utilities jar? We could add it as another project in this repo and publish it in the same maven group, but as a separate artifact. That way folks wouldn't need to include it if they didn't want to, but it would be available and very seamless to integrate in.
Also fine with me.
What are your thoughts on creating a custom Map implementation that is a NestedMap instead of just using nested semantics within an existing Collections Map class?
On Mon, Dec 8, 2014 at 4:48 PM, Kevin Litwack notifications@github.com wrote:
What would you guys think about creating a separate event utilities jar? We could add it as another project in this repo and publish it in the same maven group, but as a separate artifact. That way folks wouldn't need to include it if they didn't want to, but it would be available and very seamless to integrate in.
— Reply to this email directly or view it on GitHub https://github.com/keenlabs/KeenClient-Java/issues/30#issuecomment-66193889 .
Definitely open to that sort of thing :)
In the 3.0 changes, we could consider accepting some interface besides a Map for the events... something like:
Set<String[]> getKeys();
Object getValue(String[] keyPath);
Kind of brainstorming here. There are downsides but it would open up some interesting possibilities.
Actually I suppose we could always have the utility library provide an interface like that for writing, but still implement the plain old Map interface for reading. That's probably the best of both worlds. It'll take a bit of effort to make the implementation efficient but it should be doable.
Interesting ideas! I think there are definitely some options there that could make things easier / simpler for the user of the SDK, specifically around the requirements for the Map.
Implementing some conversion utilities for 2.x and gathering community feedback would help to shape the design for 3.0.
On Mon, Dec 8, 2014 at 5:36 PM, Kevin Litwack notifications@github.com wrote:
Actually I suppose we could always have the utility library provide an interface like that for writing, but still implement the plain old Map interface for reading. That's probably the best of both worlds. It'll take a bit of effort to make the implementation efficient but it should be doable.
— Reply to this email directly or view it on GitHub https://github.com/keenlabs/KeenClient-Java/issues/30#issuecomment-66201267 .
As a new user, I'm interested in the status of this discussion. Has the utility for creating nested maps been incorporated in any way to this sdk or companion products? I personally think that conversion utilities should be included, given that a Map of that sort is required. In addition, I wonder whether you would consider accepting a json string itself as an 'event' parameter, which would definitely simplify usage of this sdk (no need for conversions to maps). As a counterpart to this, I would probably also expect that an extraction query could return a json string as a result.
Unfortunately we haven't yet had a chance to formalize any utilities around this into something usable. It's still definitely on our radar but it's just been a question of engineering priorities - we haven't had the time. If anyone in the community wants to tackle this as an open source contribution, I'd be happy to work with them to make that happen ;)
As for KeenClient
itself accepting a JSON string instead of a map, it's an appealing feature to have in principle but there are a couple downsides in practice:
KeenClient
, and there are already more than I'd like (3 each of addEvent
, addEventAsync
, and queueEvent
).keen.timestamp
property, then re-serialize it back to JSON. This isn't horrible, since we do already have the KeenJsonHandler
interface to handle the parsing, but it's somewhat inefficient.So I would still kind of lean towards having the client take a Map (or some custom structured interface) and having a set of utilities to help build those Maps/interfaces, including potentially from a raw JSON string.
Your feedback is appreciated though, as it helps us understand what people would find helpful in practice. I'd definitely like to make it easier for users of the SDK to work with JSON directly, if that's what they prefer. I can't make any promises on timelines for that, but sooner or later we'll get there.
Creating deeply nested maps in Java is kind of painful (see https://news.ycombinator.com/item?id=8637447). We should provide a utility to make it easier. One possible way (which has been explicitly requested at least once or twice) would be a method like:
There are probably other good solutions.