patternjavaMinor
Keep parsing JSON response until a field becomes false using GSON
Viewed 0 times
fielduntilresponsekeepgsonfalseparsingusingjsonbecomes
Problem
I have JSON which is coming by executing a URL. I want to store all the value of
Here is the flow:
JSON response:
Working code:
```
public List getIds() {
List ids = new ArrayList<>();
try {
String url = generateUrl();
boolean hasMore = true;
while (hasMore) {
Optional response = executeUrl(url);
if (!response.isPresent()) {
JsonElement jelement = new JsonParser().parse(response.get());
JsonObject jobject = je
id field in a List by parsing through GSON. Also, if the next field is true, then that means there is another URL at the bottom of the JSON which I need to use to get the second page which will also look like this JSON response which will have ids again, so I will add those ids into the same list again.Here is the flow:
- Execute the URL and get JSON response. After, that check if the
nextfield is true or false in that JSON response.
- If it is false, then there is only one page so add all those ids into my
Listand return.
- If it is true, then there will be another URL at the bottom. Use that URL and add prefix to it and then execute it to get another JSON response and repeat the same thing until
nextis false and keep adding all ids into the same list.
JSON response:
{
"status": {
"number": "100",
"value": "ok"
},
"next": true,
"result": [{
"type": "Process",
"id": "1.2.3.4.5.6",
"device": "hello"
}, {
"type": "Process",
"id": "9.55.1.2.3.2",
"device": "world"
}, {
"type": "Process",
"id": "5.4.3.2.1.2",
"device": "proc"
}, {
"type": "Process",
"id": "99.11.11.2.3.4",
"device": "oreon"
}],
"pagination": {
"cursor": ["", "57d9e9d4e4b073aa2f401542", ""],
"limit": [10000, 1000, 1000],
"hint": 0,
"maxFetch": 0,
"skipEmptyPage": false
},
"next": {
"method": "GET",
"url": "/tries/next_set"
}
}Working code:
```
public List getIds() {
List ids = new ArrayList<>();
try {
String url = generateUrl();
boolean hasMore = true;
while (hasMore) {
Optional response = executeUrl(url);
if (!response.isPresent()) {
JsonElement jelement = new JsonParser().parse(response.get());
JsonObject jobject = je
Solution
Overall I think it is a good solution.
Exception handling
I think getIds()-method cannot handle exceptions properly in the context of any application.
Currently it makes the result look like as nothing has happened even an exception is thrown. I do not think that callers can make valid assumptions on the returned value so I suggest to encapsulate the general Exception into a RuntimeException (or some of your own). The caller of the method should know if the result is valid. If you want the caller continue to process with partial results then you can make your own exception containing the information received so far. But to abstract from errors is dangerous.
The decision of throwing exceptions is as important as catching, rethrowing, wrapping, logging and handle them.
If you throw an exception you do it because you want to indicate that the assumptions a method has are not met so it cannot fulfill its work.
If you want to rethrow an exception the purpose mostly is to intercept information.
If you wrap an exception you want to
An exception should be handled at a point it can be handled. And this is not neccessarily the method it occurs.
Logging an exception is the last option for exceptional cases that have no previous mentioned handle. And this is also not neccessarily the method it occurs.
Remove Optional-construct
To exit the loop you determine the existance of a next "url". That should be your single point of truth. "getResponse" is called if "hasMore" is true. But If "getResponse" gets called then it is called under the assumption that there "is" more. If the getResponse-method cannot fulfill the work it is an exception. My view is that getting an "Optional" when "hasMore" has determined before is contradictory.
Throw an exception and the same mechanism as described in "Exception handling" is used. It is transparent to the caller. The caller always will get all necessary information and can decide on its own what to do. Either the getIds()-method returns normal or an exception is thrown that contains the Ids received so far.
Ambigious next-key and semantic redundancy
Currently you have two next-keys holding different values: boolean and String. Theoretically a JsonObject should have unique keys. But any behaviour you rely on to retreive values from this key is specific to the JSON library you use. Maybe GSON does it right somehow.
Furthermore you have semantical redundancy. The existance of the next URL is sufficient information to determine if there is a next URL.
My suggestion is to get rid of the semantic redundancy as early as possible so that further algorithms work with a single source of truth. Maybe you have a preprocession of the Json String and return the JsonObject as the return value of your execute-method().
Reuse JsonParser
I think that is a little issue. But I would reuse the JsonParser-instance as I do not know if this is an expensive operation to instantiate this kind of object.
Extracting constants
There are code fragments that should be extracted to constants. the "key" String is repeated but adresses the same semantic. But also the other keys to access the json object.
Variable scope
The subjects become pedantic... You may reduce the scope of the "hasMore" variable by using a for loop rather than a while loop.
Types
You should also have a look at your GSON-Types. To determine the id you ask an JsonElement. A JsonElement may also be a JsonNull-Object (see here). Maybe you have to introduce some type-check.
Code
```
private static final String KEY_NEXT = "next";
private static final String KEY_URL = "url";
private static final String KEY_RESULT = "result";
private static final String KEY_ID = "id";
public List getIds() {
List ids = new ArrayList<>();
try {
String url = generateUrl();
final JsonParser jsonParser = new JsonParser();
for (boolean hasMore = true; hasMore;) {
JsonObject jobject = execute(url);
JsonArray jarray = jobject.getAsJsonArray(KEY_RESULT);
for (JsonElement type : jarray) {
ids.add(type.getAsJsonObject().get(KEY_ID).getAsString());
}
hasMore = jobject.has(KEY_NEXT); // assumption, that only the next-key with the url is possibly present
if (hasMore) {
url = PREFIX + jobject.getAsJsonObject(KEY_NEXT).get(KEY_URL).getAsString();
}
}
} catch (Exception ex) {
throw new PartialResultException(ex, ids);
}
return ids;
}
private static class PartialResultException extends RuntimeException {
private List idsReceivedSoFar;
public PartialResultException(Exception ex, List idsReceivedSoFar) {
super(ex);
this.idsReceivedSoFar = idsReceivedSoFar;
}
public List getIdsReceivedSoFar() {
return idsReceivedSoFar;
}
}
// Example cal
Exception handling
I think getIds()-method cannot handle exceptions properly in the context of any application.
Currently it makes the result look like as nothing has happened even an exception is thrown. I do not think that callers can make valid assumptions on the returned value so I suggest to encapsulate the general Exception into a RuntimeException (or some of your own). The caller of the method should know if the result is valid. If you want the caller continue to process with partial results then you can make your own exception containing the information received so far. But to abstract from errors is dangerous.
The decision of throwing exceptions is as important as catching, rethrowing, wrapping, logging and handle them.
If you throw an exception you do it because you want to indicate that the assumptions a method has are not met so it cannot fulfill its work.
If you want to rethrow an exception the purpose mostly is to intercept information.
If you wrap an exception you want to
- map the Exception to an exception of YOUR domain
- make the Exception signature irrelevant (RuntimeException)
An exception should be handled at a point it can be handled. And this is not neccessarily the method it occurs.
Logging an exception is the last option for exceptional cases that have no previous mentioned handle. And this is also not neccessarily the method it occurs.
Remove Optional-construct
To exit the loop you determine the existance of a next "url". That should be your single point of truth. "getResponse" is called if "hasMore" is true. But If "getResponse" gets called then it is called under the assumption that there "is" more. If the getResponse-method cannot fulfill the work it is an exception. My view is that getting an "Optional" when "hasMore" has determined before is contradictory.
Throw an exception and the same mechanism as described in "Exception handling" is used. It is transparent to the caller. The caller always will get all necessary information and can decide on its own what to do. Either the getIds()-method returns normal or an exception is thrown that contains the Ids received so far.
Ambigious next-key and semantic redundancy
Currently you have two next-keys holding different values: boolean and String. Theoretically a JsonObject should have unique keys. But any behaviour you rely on to retreive values from this key is specific to the JSON library you use. Maybe GSON does it right somehow.
Furthermore you have semantical redundancy. The existance of the next URL is sufficient information to determine if there is a next URL.
My suggestion is to get rid of the semantic redundancy as early as possible so that further algorithms work with a single source of truth. Maybe you have a preprocession of the Json String and return the JsonObject as the return value of your execute-method().
Reuse JsonParser
I think that is a little issue. But I would reuse the JsonParser-instance as I do not know if this is an expensive operation to instantiate this kind of object.
Extracting constants
There are code fragments that should be extracted to constants. the "key" String is repeated but adresses the same semantic. But also the other keys to access the json object.
Variable scope
The subjects become pedantic... You may reduce the scope of the "hasMore" variable by using a for loop rather than a while loop.
Types
You should also have a look at your GSON-Types. To determine the id you ask an JsonElement. A JsonElement may also be a JsonNull-Object (see here). Maybe you have to introduce some type-check.
Code
```
private static final String KEY_NEXT = "next";
private static final String KEY_URL = "url";
private static final String KEY_RESULT = "result";
private static final String KEY_ID = "id";
public List getIds() {
List ids = new ArrayList<>();
try {
String url = generateUrl();
final JsonParser jsonParser = new JsonParser();
for (boolean hasMore = true; hasMore;) {
JsonObject jobject = execute(url);
JsonArray jarray = jobject.getAsJsonArray(KEY_RESULT);
for (JsonElement type : jarray) {
ids.add(type.getAsJsonObject().get(KEY_ID).getAsString());
}
hasMore = jobject.has(KEY_NEXT); // assumption, that only the next-key with the url is possibly present
if (hasMore) {
url = PREFIX + jobject.getAsJsonObject(KEY_NEXT).get(KEY_URL).getAsString();
}
}
} catch (Exception ex) {
throw new PartialResultException(ex, ids);
}
return ids;
}
private static class PartialResultException extends RuntimeException {
private List idsReceivedSoFar;
public PartialResultException(Exception ex, List idsReceivedSoFar) {
super(ex);
this.idsReceivedSoFar = idsReceivedSoFar;
}
public List getIdsReceivedSoFar() {
return idsReceivedSoFar;
}
}
// Example cal
Code Snippets
private static final String KEY_NEXT = "next";
private static final String KEY_URL = "url";
private static final String KEY_RESULT = "result";
private static final String KEY_ID = "id";
public List<String> getIds() {
List<String> ids = new ArrayList<>();
try {
String url = generateUrl();
final JsonParser jsonParser = new JsonParser();
for (boolean hasMore = true; hasMore;) {
JsonObject jobject = execute(url);
JsonArray jarray = jobject.getAsJsonArray(KEY_RESULT);
for (JsonElement type : jarray) {
ids.add(type.getAsJsonObject().get(KEY_ID).getAsString());
}
hasMore = jobject.has(KEY_NEXT); // assumption, that only the next-key with the url is possibly present
if (hasMore) {
url = PREFIX + jobject.getAsJsonObject(KEY_NEXT).get(KEY_URL).getAsString();
}
}
} catch (Exception ex) {
throw new PartialResultException(ex, ids);
}
return ids;
}
private static class PartialResultException extends RuntimeException {
private List<String> idsReceivedSoFar;
public PartialResultException(Exception ex, List<String> idsReceivedSoFar) {
super(ex);
this.idsReceivedSoFar = idsReceivedSoFar;
}
public List<String> getIdsReceivedSoFar() {
return idsReceivedSoFar;
}
}
// Example call
public void caller() {
List<String> ids = new ArrayList<>();
try {
ids.addAll(getIds());
} catch (PartialResultException e) {
// omit this if the caller is not interested in the ids received so far if an exception occured
ids.addAll(e.getIdsReceivedSoFar());
}
// code that uses the ids
for (String string : ids) {
...
}
}private JsonObject execute(final String url) {
try (AsyncHttpClient asyncHttpClient = new DefaultAsyncHttpClient()) {
Future<Response> future = asyncHttpClient.prepareGet(url).execute();
Response response = future.get();
final JsonParser jsonParser = new JsonParser();
JsobObject jsonResponse = jsonParser.parse(response.getResponseBody(StandardCharsets.UTF_8.displayName())).getAsJsonObject();
removeNextKeyBooleanValue(jsonParser);
return jsonResponse;
} catch (IOException | InterruptedException | ExecutionException ex) {
throw new RuntimeException(ex);
}
}private JsonObject execute(final String url) throws IOException, InterruptedException, ExecutionException {
try (AsyncHttpClient asyncHttpClient = new DefaultAsyncHttpClient()) {
Future<Response> future = asyncHttpClient.prepareGet(url).execute();
Response response = future.get();
final JsonParser jsonParser = new JsonParser();
JsobObject jsonResponse = jsonParser.parse(response.getResponseBody(StandardCharsets.UTF_8.displayName())).getAsJsonObject();
removeNextKeyBooleanValue(jsonParser);
return jsonResponse;
}
}Context
StackExchange Code Review Q#155070, answer score: 5
Revisions (0)
No revisions yet.