patternjavaMinor
Parsing a string with named sections and a key-value pair on each line
Viewed 0 times
keyeachlinewithnamedvalueparsingandstringpair
Problem
I have a response String as shown below which I need to parse it and store it my class. Format is shown below:
Below is the response string.
Below is my class:
In the above class,
I'd like a code review to see whether this can be improved in any way?
- task name followed by this dotted line
-------------which is also fixed
- and then just below
key:valuepair. It can have many key value pair
Below is the response string.
abc-------------
Load:79008
Peak:4932152
def-------------
Load:79008
Peak:4932216
ghi-------------
Load:79008
Peak:4874588
pqr-------------
Load:79008
Peak:4874748Below is my class:
public class NameMetrics {
private String name;
private Map metrics;
// setters and getters
}In the above class,
name should be abc and metrics map should have Load as the key and 79008 as the value and same with other key:value pairs. Below is the code I have.String response = restTemplate.getForObject(url, String.class);
BufferedReader reader = new BufferedReader(new StringReader(response));
NameMetrics current = null;
Map metricsHolder = null;
List result = new ArrayList<>();
while (true) {
String s = reader.readLine();
if (s == null) {
break; // end reached
}
if (s.trim().isEmpty()) {
continue; // Skip empty line
}
int cut = s.indexOf(':');
if (cut == -1) {
cut = s.indexOf('-');
if (cut == -1) {
continue;
}
metricsHolder = new HashMap();
current = new NameMetrics();
current.setName(s.substring(0, cut));
result.add(current);
} else if (current != null) {
metricsHolder.put(s.substring(0, cut), s.substring(cut + 1));
current.setMetrics(metricsHolder);
}
}I'd like a code review to see whether this can be improved in any way?
Solution
First, I'd like to point out that if are you storing this from your program and reading it back from a file later, you'd be better off Serializing it. But since you're being given this file, let's move on.
Infinite Loops
It's generally discouraged to use
Initializing variables as
I also see you initialized a few variables as null, but that isn't necessary since it's implied for classes. (Also note primitive variables are implied as 0 during initialization).
Using the output format to your advantage
You also have a
Putting it all together
Handling varying keys and names
When there are a variable number of keys, we still know that they're all generally in the same format of the name followed dashes, then a list of
Infinite Loops
It's generally discouraged to use
while(true) loops. It's not particularly bad (in fact it's sometimes better), but it's always best to look for an alternative for readability sake. Instead, let's use while ((line = reader.readLine()) != null).Initializing variables as
nullI also see you initialized a few variables as null, but that isn't necessary since it's implied for classes. (Also note primitive variables are implied as 0 during initialization).
Using the output format to your advantage
You also have a
cut variable that gets the index of the tokens, and then skips the line if it's blank. However, we know that the output is always going to be grouped into three lines, and we always know what those lines are going to be. We also know the data you really want is going to be the first three characters for the name, and everything after the first five characters for the tokens. Let's use that and make everything clear!Putting it all together
String response = restTemplate.getForObject(url, String.class);
BufferedReader reader = new BufferedReader(new StringReader(response));
NameMetrics current;
Map metricsHolder;
List result = new ArrayList<>();
String name, load, peak;
String nameline;
while ((nameline = reader.readLine()) != null) {
name = nameLine.substring(0, 3);
load = reader.readLine().substring(5);
peak = reader.readLine().substring(5);
reader.readline(); //eat empty line
current = new NameMetrics();
metricsHolder = new HashMap<>(); //No need to repeat the stuff inside the diamond operator
metricsHolder.put("Load", load);
metricsHolder.put("Peak", peak);
current.setName(name);
current.setMetrics(metricsHolder);
result.add(current);
}Handling varying keys and names
When there are a variable number of keys, we still know that they're all generally in the same format of the name followed dashes, then a list of
key:value lines, then a blank line. We can still use this to our advantage as follows:String name;
String[] mapValues;
final int KEY = 0, VALUE = 1;
String nameline, keyLine;
while ((nameline = reader.readLine().trim()) != null) { //Loop until the end of the file
name = nameLine.substring(0, nameLine.indexOf("-") + 1); //Plus because substring excludes last index
metricsHolder = new HashMap<>(); //No need to repeat the stuff inside the diamond operator
while (!(keyLine = reader.readLine().trim()).isEmpty()) { //Loop until block ends with an empty line
mapValues = reader.readLine().split(":"); //Split key:value format
metricsHolder.put(mapValues[KEY], mapValues[VALUE]);
}
current = new NameMetrics();
current.setName(name);
current.setMetrics(metricsHolder);
result.add(current);
}Code Snippets
String response = restTemplate.getForObject(url, String.class);
BufferedReader reader = new BufferedReader(new StringReader(response));
NameMetrics current;
Map<String, String> metricsHolder;
List<NameMetrics> result = new ArrayList<>();
String name, load, peak;
String nameline;
while ((nameline = reader.readLine()) != null) {
name = nameLine.substring(0, 3);
load = reader.readLine().substring(5);
peak = reader.readLine().substring(5);
reader.readline(); //eat empty line
current = new NameMetrics();
metricsHolder = new HashMap<>(); //No need to repeat the stuff inside the diamond operator
metricsHolder.put("Load", load);
metricsHolder.put("Peak", peak);
current.setName(name);
current.setMetrics(metricsHolder);
result.add(current);
}String name;
String[] mapValues;
final int KEY = 0, VALUE = 1;
String nameline, keyLine;
while ((nameline = reader.readLine().trim()) != null) { //Loop until the end of the file
name = nameLine.substring(0, nameLine.indexOf("-") + 1); //Plus because substring excludes last index
metricsHolder = new HashMap<>(); //No need to repeat the stuff inside the diamond operator
while (!(keyLine = reader.readLine().trim()).isEmpty()) { //Loop until block ends with an empty line
mapValues = reader.readLine().split(":"); //Split key:value format
metricsHolder.put(mapValues[KEY], mapValues[VALUE]);
}
current = new NameMetrics();
current.setName(name);
current.setMetrics(metricsHolder);
result.add(current);
}Context
StackExchange Code Review Q#127238, answer score: 5
Revisions (0)
No revisions yet.