Problem
My current implementation,
class Person {
int id;
String name;
LocalDate start;
LocalDate end;
/*getters and setters*/
...
}
private void checkForTimespanOverlaps(List<Person> persons)
{
final int totalPersons = superiorSalesAgentHasFunctionDTOs.size();
LocalDate[][] timespans = new LocalDate[totalPersons][2];
int row = 0;
for (Person person : persons)
{
timespans[row][0] = person.getStartDate();
timespans[row][1] = person.getEndDate();
row++;
}
// sort timespans array
LocalDate[][] sortedTimespans = sortTimespan(timespans);
for (int row = 0; row < sortedTimespans.length - 1; row++)
{
if ((sortedTimespans[row][1].isAfter(sortedTimespans[row + 1][0])))
{
//there is overlap as previous person's endDate is after next person's startDate
}
}
}
private LocalDate[][] sortTimespan(LocalDate[][] timespans)
{
Arrays.sort(timespans, new Comparator<LocalDate[]>()
{
@Override
public int compare(LocalDate[] startDates, LocalDate[] endDates)
{
LocalDate startDate = startDates[0];
LocalDate endDate = endDates[0];
return startDate.compareTo(endDate);
}
});
return supervisionTimespans;
}
Is it Good solution?
or how can it be Refactored and make Clean Code?
Solution
You’re doing way too much work here. You don’t need the arrays at all.
First, write a Comparator<Person>
. It’s better off in its own class so it can be reused later.
import java.util.Comparator;
final class TimespanComparator implements Comparator<Person> {
@Override
public int compare(final Person person1, final Person person2) {
return person1.getStartDate().compareTo(person2.getEndDate());
}
}
Then sort a new copy of the List<Person>
using that comparator. Walk through the sorted list to do your comparison. You can start the loop at index 1 because if there’s only 1 person, there can’t be any overlap and the loop will never get entered.
private void checkForTimespanOverlaps(final List<Person> persons) {
final List<Person> sortedList = new ArrayList<>(persons);
Collections.sort(sortedList, new TimespanComparator());
for (int i = 1; i < sortedList.size(); i++) {
final LocalDate priorEndTime = sortedList.get(i - 1).getEndDate();
final LocalDate currentStartTime = sortedList.get(i).getStartDate();
if (priorEndTime.isAfter(currentStartTime)) {
// ...
}
}
}
As an aside, American english uses ‘people’ as the plural of person, not persons. Americans might prefer to see the variable named people instead.