Data Persistence

Question

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<String, String> map = new HashMap<String, String>();
        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<String, String> 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;
    }

    public void pushData(HashMap<String, String> map) {

        String url = "https://dweet.io/dweet/for/" + thing + "?";

        for (Map.Entry<String, String> item : map.entrySet()) {
            String k = URLEncoder.encode(item.getKey());
            String v = URLEncoder.encode(item.getValue());
            String sub = k + '=' + v + "&"; url += sub;
        }

        try 
        {
            setLocation(url);
        }

        catch (Exception e) 
        {

        }

        readData();

    }

    public void pushData(String key, String value) {
        String url = "https://dweet.io/dweet/for/" + thing + "?";
        setLocation(url + key + "=" + value); readData();
    }

    public String pullData(String key) {

        setLocation("https://dweet.io/get/latest/dweet/for/" + thing);
        JSONArray main = new JSONObject(readData()).getJSONArray("with");
        JSONObject data = ((JSONObject) main.get(0)).getJSONObject("content");

        return data.optString(key);
    }

    public HashMap<String, String> pullData() {

        setLocation("https://dweet.io/get/latest/dweet/for/" + thing);
        JSONArray main = new JSONObject(readData()).getJSONArray("with");
        JSONObject data = ((JSONObject) main.get(0)).getJSONObject("content");

        HashMap<String, String> map = new HashMap<String, String>();

        for (String key : data.keySet()) {
            map.put(key, data.optString(key));
        }

        return map;
    }

}

Answer 1

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<String, String> 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<String, String> map).

Answer 2

• Off the bat, your code contains no comments. Comments are useful to know what the code does.

• Your code is very ambiguous. What is

  String thing; // a username?
  

  ... it behooves you to name this more descriptively.

  setLocation()
  

  I notice it sets the subdirectory. You can make this more specific.

• URLEncoder.encode is marked as deprecated in Netbeans 8. You'll probably want to look into a more forward-compatible alternative:

  public static String encode(String s, String enc);
  

• Strings are hardcoded all over. It's much easier to maintain code when you put all of those type of things in one place.

• You are eating your errors. This is very bad. Errors tell you what is going wrong. When you eat errors you summon the wrath of the Gods; and you will never know when your code isn't working.