patternjavaMinor
Data Persistence
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
JitterPullDemo.java
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;
}
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
You have the duplicated code block above in a few places, it's probably better to put them in a method.
Same goes for the
Input sanitization?
You are currently constructing your
Handling for one or multiple keys/values
I will prefer to convert the arguments to the
Don't swallow
I'm also not to keen on calling your
Define variables/parameters with interfaces, not implementations
Just one more thing, it's better to declare
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
ExceptionsreadData() 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.