-
Notifications
You must be signed in to change notification settings - Fork 91
Description
In the drawChart method of ArcChart, if sortByCluster is true, a TreeSet is used to order clusters:
https://github.com/HanSolo/charts/blob/jdk17/src/main/java/eu/hansolo/fx/charts/ArcChart.java#L586
// Sort items by max sum of outgoing reversed
List<PlotItem> sortedItems;
if (getSortByCluster()) {
sortedItems = new LinkedList<>();
TreeSet<Cluster> clusters = new TreeSet<>(items.stream().filter(item -> null != item.getCluster()).map(PlotItem::getCluster).collect(Collectors.toSet()));
clusters.forEach(cluster -> sortedItems.addAll(cluster.getSortedItems()));
if (sortedItems.isEmpty()) { sortedItems.addAll(items); }
} else {
sortedItems = new LinkedList<>(items);
Collections.sort(sortedItems, Comparator.comparingDouble(PlotItem::getSumOfOutgoing).reversed());
}The compareTo method of Cluster just uses getMaxValue():
https://github.com/HanSolo/charts/blob/jdk17/src/main/java/eu/hansolo/fx/charts/Cluster.java#L175
@Override public int compareTo(final Cluster other) {
if (getMaxValue() < other.getMaxValue()) {
return 1;
} else if (getMaxValue() > other.getMaxValue()) {
return -1;
} else {
return 0;
}
}As a result, multiple clusters with the same max value collide in the TreeSet and only a subset of clusters are stored. This causes a discrepancy between the anticipated total items and what is actually added to the sortedItems collection and results in a IndexOutOfBoundsException with the subsequent sortedItems.get(i) operation:
https://github.com/HanSolo/charts/blob/jdk17/src/main/java/eu/hansolo/fx/charts/ArcChart.java#L598
for (int i = 0 ; i < noOfItems ; i++) {
PlotItem item = sortedItems.get(i);
//double itemSize = Helper.clamp(minItemSize, maxItemSize, item.getSumOfOutgoing() * itemSizeFactor);
double itemX = insetX + i * stepSizeX;
double itemY = centerY;
itemPoints.put(item, new Point(itemX, itemY));
}Is there a reason that clusters are sorted via a set vs. a non-unique data structure with sorting? My workaround in my project was to subclass Cluster and override compareTo to also use name when maxValues are the same.
I am referencing code from the jdk17 branch.