Converting HashMap data to a Response object

Question

I have a HashMap and I want to convert data in it to a Response object. Is it possible to achieve this code that parses the string in a better and optimized and cleaner way, maybe by using streams?

class Converter{
 public static void main(String[] args) {
 Map<String, Long> map = new HashMap<String, Long>();
 map.put("111", 80) // first 
 map.put("1A9-ppp", 190) // second
 map.put("98U-6765", 900) // third
 map.put("999-aa-local", 95) // fourth
 List<FinalProduct> products = new ArrayList<>();
 for(String key : map.keySet()){
 FinalProduct response = new FinalProduct();
 String[] str = key.split("\\-");
 response.id = str[0] //id is always present in key i.e 111, 1A9, 98U,999
 if(str.length == 2) {
 if(str.matches("-?\\d+(\\.\\d+)?")){ // check if is numeric
 response.code = str[1]; // 6765
 }
 else{
 response.city = str[1]; //ppp
 }
 }
 if(str.length == 3){
 response.client = str[1]; //aa
 response.type = str[2]; // local
 } 
 response.qty = map.get[key];
 products.add(response);
 }
 }
}
class FinalProduct{
String id;
String city;
Long code;
String client;
String type;
Long qty;
// getters and setters
}

• pacmaninbw: It might be better if the contents of the class FinalProduct was properly indented.
• Roland Illig: @pacmaninbw And the code of the Converter class as well. That too looks ugly.
• dustytrash: What's the purpose of this? For example, where does "1A9-ppp" come from? If you have control over it, it's best not to hack a String for properties.
• hc_dev: If the code works and you want to get reviews for optimization, better post to codereview.stackexchange.com
• AJNeufeld: if(str.matches("-?\\d+(\\.\\d+)?");){ this code does not compile.

Answer 1

The only thing you need is a saparate method doing the parsing.

This parse method can either be in a specific converter class that is used in a specific case when you're taking the strings and ammounts from a certain place.

public static FinalProduct parse(String key, Long quantity) {
 //transform into FinalProduct here
}

Or if you'll always start from those business specific Strings you could just provide a constructor for FinalProduct that takes those as input params:

public class FinalProduct{
 private String id;
 private String city;
 private Long code;
 private String client;
 private String type;
 private Long qty;
 public FinalProduct(String codedProduct, Long qty) {
 this.qty = qty;
 //parse coded input string here
 } 
 // only getters here, no setters
}

Since the variables are probably not going to change after initializing the class you can make them final and not provide setters. Immutable objects are generaly easier to get right in a production code base (less edge cases when using multi threading for example).

Using streams isn't going to make much of a difference really. The parsing itself isn't going to change and the loop is already as simple as it's going to get. I personally prefer the loop version over the stream version when going through a map:

for loop:

List<FinalProduct> products = new ArrayList<>();
for (Map.Entry<String, Long> entry : map.entrySet()) {
 products.add(new FinalProduct(entry.getKey(), entry.getValue()));
}

stream:

map.entrySet().stream()
 .map(entry -> new FinalProduct(entry.getKey(), entry.getValue()))
 .collect(Collectors.toList());