Converting and enriching an array of data

Question

The method below converts an array of TheatreListData objects to an array of TheatreListDataWrapper. It also loads additional attributes such as patientIdentifierNumber and requestText to enrich the converted data.

This method is called by many EJBs, and so I'm especially looking for ways to improve performance.

/** 
* Converts TheatreListDatas into TheatreListDataWrappers. This also 
* loads the attributes that has to be generated from client such as 
* patientIdentifierNumber, requestText 
* 
* @param theatreListData 
* @return TheatreListDataWrapper[] 
*/
public TheatreListDataWrapper[] wrapData(TheatreListData[] theatreListData){
 TheatreListDataWrapper[] wrapperList = wrapDataWithoutRequestData(theatreListData);
 if (wrapperList != null && wrapperList.length > 0)
 {
 Set<String> requestIds = new HashSet<String>();
 for (TheatreListData listData : theatreListData)
 {
 // Collect request id for each List Data into a set
 String requestId = listData.getCarePlan().requestId;
 //Should check for '-1' here to avoid calls to fetch invalid requests.
 //'-1' could have been saved with registrations with 'None' requests.
 if (requestId != null && requestId.trim().length() > 0 && !requestId.equals("-1"))
 {
 //Since adding to a set no duplicates are possible.
 requestIds.add(requestId);
 }
 }
 //Load request Id, request Text map using the collected request ids.
 Map<String, String> requestMap = Collections.emptyMap();
 if (!requestIds.isEmpty())
 {
 requestMap = getRequestDelegator().getRequestPresentationStringsByIds(new ArrayList<String>(requestIds));
 }
 //Create wrappers & fill with loaded data.
 for (int i = 0; i < theatreListData.length; i++)
 {
 TheatreListDataWrapper wrapper = wrapperList[i];
 TheatreListData listData = theatreListData[i];
 String requestId = listData.getCarePlan().requestId;
 if (requestId != null && requestId.trim().length() > 0)
 {
 wrapper.requestText = requestMap.get(requestId) != null ? requestMap.get(requestId) : "";
 }
 wrapperList[i] = wrapper;
 }
 }
 return wrapperList;
 }

Answer 1

Depending on the constant overhead of retrieving the request presentation string for a request ID (i.e. is it considerably more efficient to get all the strings for a bunch of IDs at once than for each ID separately?), a better design might be to iterate over the theatre list only once, getting the representation strings as you go and caching the results in a Map.

Map<String, String> cache = new HashMap<>();
for (int i = 0; i < theatreListData.length; i++) {
 TheatreListDataWrapper wrapper = wrapperList[i];
 TheatreListData listData = theatreListData[i];
 String requestId = listData.getCarePlan().requestId;
 if (validRequestId(requestId)) {
 String text = cache.get(requestId);
 if (text == null) {
 text = getRequestDelegator().getRequestPresentationStringsById(requestId);
 cache.put(requestId, text);
 }
 wrapper.requestText = text;
 }
}

This is probably not a good idea of getting the request text requires, say, a database query.

Other than that, I can think of no algorithmic improvements. You should store the result of requestMap.get(requestId) in a local variable so you don't do the call twice; that's inefficient. Perhaps you can modify getRequestPresentationStringsByIds to take a Collection instead of a List so that you don't have to wrap your set in an ArrayList. Can't think of more.

Answer 2

If you're under Java 8 then I'd prefer using the Stream API:

theatreListData
 .stream()
 .map(listData -> listData.getCarePlan().requestId)
 .filter(id -> id != null)
 .filter(id -> id.trim().length() > 0)
 .filter(id -> !id.equals("-1"))
 .collect(Collectors.toSet())

And this comparsion never throws NPE.

"Constant".equals(variable);

• Jude Niroshan: Sorry mate. I'm in Java 7
• llama: Then I would define each "for" as new function