HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Data Persistence

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
datapersistencestackoverflow

Problem

I was wondering if I could get some feedback on the library I created to persist data online.

JitterPushDemo.java

public class JitterPushDemo {

    public static void main(String[] args) {

        Jitter jitter = new Jitter("scores");

        // create hash map to map scores to players

        HashMap map = new HashMap();
        map.put("Bob", "300");
        map.put("Sam", "500");

        // push data online to jitter object
        jitter.pushData(map);

    }

}


JitterPullDemo.java

public class JitterPullDemo {

    public static void main(String[] args) {

        Jitter jitter = new Jitter("scores");

        // now pulling data from online and iterating
        for (Map.Entry item: jitter.pullData().entrySet()) {
            System.out.println("Player: " + item.getKey());
            System.out.println("Scores: " + item.getValue());
        }  

    }

}


Source Code

```
package org.main;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URL;
import java.net.URLEncoder;
import java.util.HashMap;
import java.util.Map;

import org.json.JSONArray;
import org.json.JSONObject;

public class Jitter {

URL location;
String thing;

public Jitter(String thing) {
this.thing = thing;
}

private void setLocation(String place) {

try
{
location = new URL(place);
location.openConnection();
}

catch (Exception e)
{

}

}

public String readData() {

InputStream dataStream = null;
InputStreamReader read = null;
String result = ""; int value;

try
{
dataStream = location.openStream();
read = new InputStreamReader(dataStream);

while ((value = read.read()) != -1)
result += String.valueOf((char) value);
}

catch (Exception e)
{

}

return result;
}

Solution

FGITW:

Duplication

JSONArray main = new JSONObject(readData()).getJSONArray("with");
JSONObject data = ((JSONObject) main.get(0)).getJSONObject("content");


You have the duplicated code block above in a few places, it's probably better to put them in a method.

private JSONObject getJSONObject(final String jsonOutput) {
    return ((JSONObject) new JSONObject(jsonOutput).getJSONArray("with").get(0))
        .getJSONObject("content").
}


Same goes for the String https://dweet.io, extract it to a final field perhaps?

Input sanitization?

You are currently constructing your URLs directly by appending thing. I'm not sure how dweet handles weird keys/data, but you should consider doing some kind of sanity check here.

Handling for one or multiple keys/values

I will prefer to convert the arguments to the pushData(String, String) method as a Map so that I can pass it to the other pushData(Map) method. This ensures your URI encoding is handled correctly - as you can see currently you aren't handling that for pushData(String, String). Also, I might be missing something here, but why do you need to readData() after pushing data to the web service?

Don't swallow Exceptions

readData() does not appear to handle thrown Exceptions in any meaningful way. Should it log to some console output? Fail fast? Wrap the Exception and throw it to the caller?

I'm also not to keen on calling your InputStreamReader instance read, because read.read() sounds... strange. Why not just call it reader? And I think you'll need to close() it when you're done with it.

Define variables/parameters with interfaces, not implementations

Just one more thing, it's better to declare Map map = new HashMap<>(); (>= Java 7 syntax), since your code should not need to be aware of the actual implementation of map. Same goes for public void pushData(Map map).

Code Snippets

JSONArray main = new JSONObject(readData()).getJSONArray("with");
JSONObject data = ((JSONObject) main.get(0)).getJSONObject("content");
private JSONObject getJSONObject(final String jsonOutput) {
    return ((JSONObject) new JSONObject(jsonOutput).getJSONArray("with").get(0))
        .getJSONObject("content").
}

Context

StackExchange Code Review Q#83699, answer score: 5

Revisions (0)

No revisions yet.