Check if all objects have the property value of all other objects

Question

I have a list of objects. These objects have a processname, amount and a date. I need every object to have all dates (if it doesnt exist add a new one with amount 0). So if I have 2 processnames: 'Inflate' and 'Deflate' and they have different dates: 'Inflate':12-01-2017 and 'Deflate': 13-01-2017. I need them to have the other object's dates.

I have the following code but it feels devious:

 HashSet<Date> dates = new HashSet<>();
 HashSet<String> processes = new HashSet<>();
 //Get all the unique values from the objects. processAmounts is the list of objects
 for (ProcessAmount processAmount : processAmounts) {
 dates.add(processAmount.getDate());
 processes.add(processAmount.getProcessName());
 }
 //Loop through processnames
 for (String s : processes) {
 //Loop through all the unique dates
 for(Date date : dates) {
 //loop through the objects we have and check if the date exists.
 boolean dateFound = false;
 for(ProcessAmount processAmount : processAmounts) {
 if(processAmount.getProcessName() != null && processAmount.getProcessName().equals(s)) {
 if(processAmount.getDate().equals(date)) {
 dateFound = true;
 }
 }
 }
 //If the date is not found add a new object with amount 0
 if(!dateFound) {
 processAmounts.add(new ProcessAmount(date, 0, s));
 }
 }
 }

It might be a bit unclear for some people so if you don't understand I will try to explain it another way.

• Timothy Truckle: "These objects have a processname, amount and a date. I need every object to have all dates" does all actually mean both?
• Timothy Truckle: what is the object your explanation refers to?
• Timothy Truckle: " 'Inflate':12-01-2017 and 'Deflate': 13-01-2017. I need them to have the other object's dates." Does this mean you want 'Inflate': 13-01-2017 and 'Deflate': 12-01-2017?
• LVK: @TimothyTruckle Yes exactly. I need all objects to have all the dates which exists in the list. So in this case I would end up with 4 objects.
• LVK: I don't want to change the date btw. I want to add a new object with the correct date and 0 as amount.

Answer 1

Code Improvements

I'll remain in original Java-7 style here, but some of the items might be expressed definitely better in functional Java-8 style.

Use Interfaces

HashSet<Date> dates = new HashSet<>();

In declarations, replace collection implementation types with interfaces:

Set<Date> dates = new HashSet<>();

It's just a cleaner practice: prefer interfaces over implementations.

Bug or Suspicious Comparison of Dates

The old java.util.Date are compared with equals(), which will lead to unpredictable behavior, because they are compared up to milliseconds. So two Dates created within even a short delay will not be equal. Just try this simple test:

@Test
public void compareDates() throws InterruptedException {
 Date date1 = new Date();
 Thread.sleep(50L);
 Date date2 = new Date();
 assertNotEquals(date1, date2); // they are not equal, although `toString()` will output the same value.
}

In the current code this looks like a bug: there is a high risk that multiple ProcessAmount(date, 0, s) are created for same date and dateFound is hard to ever become true in the loop.

"Java <= 7" solutions: 1) compare them through string representations, using formatted date output with pattern up to the searched granularity ("yyyy-MM-dd"); 2) compare year-month-day values; 3) ... other multiple inventive ways found on SO.

Recommended solution: throw java.util.Date away and use LocalDate from Java 8 or Joda-Time.

Conditions

This one

if(processAmount.getProcessName() != null && processAmount.getProcessName().equals(s)) { ...

looks heavy. In the original code s looks like it should not be nullable, so

if (s.equals(processAmount.getProcessName())) {

Or use

if (Objects.equals(s, processAmount.getProcessName())) { // supports nullability

Design Improvements

Three nested for loops and conditionals create too much complexity. Really, too much.

The idea for improvement is:

1) Create a Map that associates a process name with its date. It will help us to know that the process having this name is associated with this date and that it needs to be associated with all the other known dates.

2) Iterate on the entries of the map. For each entry, create a collection of all unique dates and remove from it the date of the entry. Create a new ProcessAmount for all those dates and add it to the target list.

Here is how the whole solution might look like. Date is replaced with LocalDate:

Map<String, LocalDate> procNameToDate = new HashMap<>();
for (ProcessAmount processAmount : processAmounts) {
 procNameToDate.put(processAmount.getProcessName(), processAmount.getDate());
}
for (Map.Entry<String, LocalDate> nameToDateTuple : procNameToDate.entrySet()) {
 Collection<LocalDate> otherDates = new HashSet<>(procNameToDate.values());
 otherDates.remove(nameToDateTuple.getValue());
 for (LocalDate date : otherDates) {
 processAmounts.add(new ProcessAmount(date, 0, nameToDateTuple.getKey()));
 }
}

Answer 2

From the question and the comment I'd argue that your object model needs refactoring. There should be an Action object (you might come up with a better name...) tat contains the date and both of the processes Inflate and Deflate (not having a date themselves).

However...

This is how I would approach this by avoiding to iterate processAmounts excessively

public void fixMissingProcesses(){
 Map<String,Map<Date,ProcessAmount>> processesPerName = separateProcessesByName(); 
 List<String> processNames = new ArrayList<>(processesPerName.keySet());
 for(int i=1;i<processNames.length();i++)
 for(int j=0;j<i;j++)
 addMissingProcesses(processNames.get(i),processNames.get(j), processesPerName);
} 
private void separateProcessesByName(){
 Map<String,Map<Date,ProcessAmount>> processesPerName = new HasMap<>(); 
 for (ProcessAmount processAmount : processAmounts) {
 if(null!=processAmount.getProcessName()){
 Map<Date,ProcessAmount> processesPerDate= processesPerName.getOrDefault(processAmount.getProcessName(),new HashMap<>());
 processesPerDate.put(processAmount.getDate(),processAmount);
 processesPerName.put(processAmount.getProcessName(),processesPerDate)
 }
 }
 return processesPerName;
} 
private void addMissingProcesses(String sourceProcessName, String targetProcessName, Map<String,Map<Date,ProcessAmount>> processesPerName);
 for(Date checkDate: processesPerName.get(sourceProcessName).keySet()){
 if(!processesPerName.get(targetProcessName).containsKey(checkDate)){
 processAmounts.add(new ProcessAmount(checkDate, 0, targetProcessName)
 }
 }
}

• LVK: The amount of object's is not static though. So I will still need to loop in the first method.
• Timothy Truckle: Maybe, but only over the combinations of distinct object names... updated the answer.