utility class for reading in JSON

Question

I'm reasonably happy with the execution:

thufir@dur:~/eclipse-workspace/jsr374$ 
thufir@dur:~/eclipse-workspace/jsr374$ gradle runShadow
> Task :runShadow
Oct 22, 2017 11:59:02 AM net.bounceme.dur.json.JsonOperations tryJsonFromUrl
INFO: https://my-json-server.typicode.com/typicode/demo/db
Oct 22, 2017 11:59:03 AM net.bounceme.dur.json.Main json
INFO: {"posts":[{"id":1,"title":"Post 1"},{"id":2,"title":"Post 2"},{"id":3,"title":"Post 3"}],"comments":[{"id":1,"body":"some comment","postId":1},{"id":2,"body":"some comment","postId":1}],"profile":{"name":"typicode"}}
BUILD SUCCESSFUL in 2s
6 actionable tasks: 1 executed, 5 up-to-date
thufir@dur:~/eclipse-workspace/jsr374$ 

am less than thrilled with the variable names, error handling, and, specifically, the convertStreamToString method.

package net.bounceme.dur.json;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringReader;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Scanner;
import java.util.logging.Logger;
import javax.json.Json;
import javax.json.JsonObject;
import javax.json.JsonReader;
public class JsonOperations {
 private static final Logger LOG = Logger.getLogger(JsonOperations.class.getName());
 private JsonObject readJsonFromString(String jsonString) {
 JsonReader jsonReader = Json.createReader(new StringReader(jsonString));
 JsonObject jsonObject = jsonReader.readObject();
 return jsonObject;
 }
 private String convertStreamToString(InputStream is) {
 Scanner scanner = null;
 scanner = new java.util.Scanner(is);
 scanner = scanner.useDelimiter("\\A");
 return scanner.hasNext() ? scanner.next() : "";
 }
 public JsonObject tryJsonFromUrl(String urlString) {
 LOG.info(urlString);
 JsonObject jsonObject = null;
 try {
 jsonObject = jsonFromUrl(urlString);
 } catch (IOException e) {
 // e.printStackTrace();
 LOG.severe(e.toString());
 }
 return jsonObject;
 }
 private JsonObject jsonFromUrl(String urlString) throws MalformedURLException, IOException {
 LOG.fine(urlString);
 InputStream inputStream = new URL(urlString).openStream();
 String jsonString = convertStreamToString(inputStream);
 inputStream.close();
 LOG.fine(jsonString);
 JsonObject jsonObject = readJsonFromString(jsonString);
 return jsonObject;
 }
}

Open, really, to any input. It's meant as a generic sort of utility. Perhaps static methods? Wasn't sure.

Finally, it's perhaps an erroneous assumption that the JSON will come in via URL...

Basically, just want to make it a bit more robust. For example, if there's a problem, pretty much anywhere down the line...perhaps throw a custom exception? Or return a default empty JSON object? Both?

Finally, if there's a library which handles URL -> JSON seamlessly then all the better.

Answer 1

To be honest: I fail to see how this class is particularly useful (at least yet).

You can immediately create a JsonReader from a URL, making your whole problematic convertStreamToString method (as well as readJsonFromString) superfluous. So far your class is equivalent to:

private JsonObject tryJsonFromUrl(String url) {
 try (InputStream is = new URL(url).openStream();
 JsonReader reader = Json.createReader(is);) {
 return reader.readObject();
 } catch (IOException e) {
 LOG.log(Level.SEVERE, e.toString(), e);
 return null;
 }
}

---

That being said, let me cover your specific questions and a few unspecific observations nonetheless, since you wanted all kinds of input.

Variable names

There is nothing wrong with your variable names IMHO.

---

Error handling

Your code may fail to close inputStream in case of an exception, leading to a potential resource leak. To prevent this, either put inputStream.close(); in a finally block or - even better - use try-with-resources as seen above.

Your code also throws a NullPointerException when urlString is null. You can add an explicit check or document that behaviour, but you should do one of them.

Error handling (2): Logging

When encountering an exception you usually want to log the stack trace as well to help with debugging. Your current logging will print something like that:

SEVERE: java.net.UnknownHostException: some-nonexisting.host.com

This may be enough for you now since you know that there is only one possible cause within your code. But think of someone else using your class or more complex scenarios, where it might be relevant to see from where your method was called: Your current logging output won't tell you anything about that.

On the other hand

LOG.log(Level.SEVERE, e.toString(), e);

will include the whole stacktrace, making it easier to trace back the error from the logging output alone (you won't be able to debug running production servers):

SEVERE: java.net.UnknownHostException: some-nonexisting.host.com
java.net.UnknownHostException: some-nonexisting.host.com
 at java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:184)
 at java.net.PlainSocketImpl.connect(PlainSocketImpl.java:172)
 at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
 at java.net.Socket.connect(Socket.java:589)
 at sun.security.ssl.SSLSocketImpl.connect(SSLSocketImpl.java:668)
 at sun.security.ssl.BaseSSLSocketImpl.connect(BaseSSLSocketImpl.java:173)
 at sun.net.NetworkClient.doConnect(NetworkClient.java:180)
 at sun.net.www.http.HttpClient.openServer(HttpClient.java:432)
 at sun.net.www.http.HttpClient.openServer(HttpClient.java:527)
 at sun.net.www.protocol.https.HttpsClient.<init>(HttpsClient.java:264)
 at sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:367)
 at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:191)
 at sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1104)
 at sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:998)
 at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:177)
 at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1512)
 at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1440)
 at sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:254)
 at java.net.URL.openStream(URL.java:1038)
 at net.bounceme.dur.json.JsonOperations.tryJsonFromUrl(JsonOperations.java:32)
 at net.bounceme.dur.json.JsonOperations.main(JsonOperations.java:25)

Here you would see that the exception was thrown in line 32 of the JsonOperations class within the tryJsonFromUrl method and that this method was called from line 25 in the main method. That gives you already a first idea of what might have led to the exception.

---

convertStreamToString method

Scanner scanner = null;
scanner = new java.util.Scanner(is);

Can be shortened to

Scanner scanner = new Scanner(is);

(you are already importing java.util.Scanner)

Otherwise it doesn't look bad (see also this Stack Overflow answer).

---

Perhaps static methods?

If it's intended to be a pure utility class, I'd go with static methods. See this Stack Overflow question for a few details.

---

Assumption that the JSON will come in via URL

For a general purpose utility class that is almost certainly an erroneous assumption. You can either offer several entry methods, e.g.

• public JsonObject tryJsonFromUrl(String urlString);
• public JsonObject tryJsonFromUrl(URL url);
• public JsonObject tryJsonFromString(String jsonString);

or a generic method, letting the caller handle the details:

• public JsonObject tryJson(InputStream is);

The latter may be a bit more work for the caller, but in return offers more flexibility (it's also the option that has been chosen for javax.json.Json).

---

Custom exception? Or return a default empty JSON object?

Whether you throw an exception, return null or return an "empty" object is to some extent a matter of style. In your case I'd probably go without an exception as the method name ("try...") suggests that reading may fail (thus making it a non-exceptional case). And since I can't think of any general use for a default object, null would be fine as well.

If you decide to throw an exception, I'd prefer a custom one to a) make your api consistent and b) abstract from implementation details (e.g. reading JSON from a String won't throw an IOException).

---

General observations

"I find your lack of javadoc disturbing".

Especially when writing code for others, your class should describe what it does. In detail. See it as a contract: Your javadoc describes what behaviour a caller can (and cannot) expect. Oracles document on javadoc has a lot more details on that.

A library which handles URL -> JSON seamlessly

The default implementation, see above ;-)

• Thufir: all excellent, thanks. would you expand a bit or clarify distinction between logging as how I had it versus the stack trace. What's the defining advantage or distinction? I thought it seemed a complex way to do a simple thing..LOL.
• Marvin: Yes, I was also a bit surprised to see no version of LOG.severe accepting a Throwable. But I don't think I've ever not used a separate logging framework (see e.g. log4j). Does the expanded part now answer your question?
• Thufir: yup. thanks for explicitly explaining. Firebase uses SL4J, so, to get into mbaas I'll have to switch anyhow.