Events client #16

Merged
merged 10 commits into from Feb 24, 2013

Conversation

Projects
None yet
2 participants
Contributor

Kami commented Feb 21, 2013

W.I.P. not ready yet

Kami added some commits Feb 21, 2013

@@ -61,7 +63,10 @@ private Object processResponse() throws IOException, ValidationException {
data = EntityUtils.toString(entity);
if (this.parseAsJson && this.responseType != null) {
- data = new Gson().fromJson(data.toString(), this.responseType);
+ GsonBuilder builder = new GsonBuilder();
@gdusbabek

gdusbabek Feb 21, 2013

Contributor

The Gson instance ought to be static.

EDIT: nm. I see that it can't be.

@gdusbabek

gdusbabek Feb 21, 2013

Contributor

If there are only going to be a limited number of response types, it makes sense to use a factory or factories that have access to a static Gson instance. Creating a new builder to create a new instance on every response is wasteful.

Gson instances are threadsafe.

+import java.util.Map;
+
+public class EventsClient extends BaseClient {
+ private static final ArrayList VALID_EVENT_TYPES = new ArrayList<String>(Arrays.asList(new String[]{
@gdusbabek

gdusbabek Feb 21, 2013

Contributor

private static final List VALID_EVENT_TYPES = Arrays.asList(new String[] {...});

+ JsonObject jsonObject;
+
+ // TODO: This is nasty - propagate type here
+ if (json.getClass() == JsonObject.class) {
@gdusbabek

gdusbabek Feb 21, 2013

Contributor

json.getClass().equals(JsonObject.class) is a little bit safer. We don't have to deal with multiple class loaders, but just sayin'.

Contributor

gdusbabek commented Feb 21, 2013

Looks ok to me. +1

Kami added a commit that referenced this pull request Feb 24, 2013

@Kami Kami merged commit c9c7829 into master Feb 24, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment