trjhtr

The method takes two parameters. If I call it with two parameters

first : ["0-1000:2","1000-2000:3"]

second: initial

then these are the expected outputs:

1. 
   

   First range:

   
   
   

   rangeId: 0-1000,
   from: 0,
   to: 1000,
   issuingRange: initial,
   minUsers: 2
   

   
   


2. 
   

   Second range:

   
   
   

   rangeId: 1000-2000
   from: 1000
   to: 2000
   issuingRange: initial
   minUsers: 3
   

   
   

This code is working but I would like to improve it. Can I use streams in Java 8? Any other change or suggestion can make the code better?

private List<Range> fillRanges(Object roleList, String issuingRange) 

if (roleList != null) 
 List<String> rangeList2 = null;
 if (roleList instanceof ArrayList<?>) 

 rangeList2 = (ArrayList<String>) roleList;
 else if (roleList instanceof String) 
 rangeList2 = new ArrayList(Arrays.asList(roleList));
 

 for (int i = 0; i < ((ArrayList) rangeList2).size(); i++) 
 String rangeId = "";
 String from = "";
 String to = "";
 String minUsers = "";
 String info = ((ArrayList) rangeList2).get(i).toString();

 logger.debug("info: " + info);

 if (info.indexOf(":") > -1) 
 rangeId = info.substring(0, info.indexOf(":"));
 minUsers = info.substring(info.indexOf(":") + 1);
 if (info.indexOf("-") > -1) 
 from = info.substring(0, info.indexOf("-"));
 to = info.substring(info.indexOf("-")+1, info.indexOf(":"));
 
 else 
 rangeId = info;
 if (info.indexOf("-") > -1) 
 from = info.substring(0, info.indexOf("-"));
 to = info.substring(info.indexOf("-")+1);
 
 minUsers = "0";
 

 ranges.add(new Range(rangeId, from, to, issuingRange, minUsers));
 

return ranges;

Your function seems to be abusing the roleList parameter. There are two modes in which this function can be called: with a single String (in which case one Range is added), or with an ArrayList<String> (in which case one Range is added for each element of the list). If you call it with any other kind of Object, then the code crashes with a NullPointerException. That's not good design.

Rather, you should define two separate functions to handle those two modes of operation. The list-handling version simply chains to the string-handling version of the function.

private void fillRanges(List<String> roleSpecs, String issuingRange) 
 for (String roleSpec : roleSpecs) 
 this.fillRanges(roleSpec, issuingRange);
 


private void fillRanges(String roleSpec, String issuingRange) 
 …

To implement the latter function, I would avoid slicing and dicing using substrings. Since this is a parsing problem that involves pattern recognition, I would use a regular expression instead to capture the substrings of interest.

private static final Pattern RANGE_REGEX = Pattern.compile(
 "^(?<rangeId>(?<from>\d*)-?(?<to>\d*))(?::(?<minUsers>\d+))?$"
);

private void fillRanges(String roleSpec, String issuingRange) 
 logger.debug("Role: " + roleSpec);
 Matcher m = RANGE_REGEX.matcher(roleSpec);
 if (!m.matches()) 
 throw new IllegalArgumentException(roleSpec);
 
 ranges.add(new Range(
 m.group("rangeId"),
 m.group("from"),
 m.group("to"),
 issuingRange,
 m.group("minUsers") == null ? "0" : m.group("minUsers")
 ));