From 8e52ea3752bc3d0cc7b5d1f17162a37e6e7f9529 Mon Sep 17 00:00:00 2001 From: Lee McLaughlin Date: Tue, 25 May 2021 15:17:18 +0100 Subject: [PATCH] Fixes 4641 & 4645 The use of Swift Algorithms partitioningIndex with exact matching clause == can provide incorrect values. It has been modfied to use a >= clause as well followed by a check for an exact match from that index. This should maintain the intended behaviours and I have added a It has been modfied to use a >= clause as well followed by a check for an exact match from that index. This should maintain the intended behaviours and I have added a fix for fetching the closest X value index when the value falls between 2 indexs and uses .closest for rounding method. --- .../Standard/ChartDataSet.swift | 19 +++++++-- Tests/ChartsTests/ChartDataTests.swift | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/Source/Charts/Data/Implementations/Standard/ChartDataSet.swift b/Source/Charts/Data/Implementations/Standard/ChartDataSet.swift index 703c0abe72..e9a3a9b0b3 100644 --- a/Source/Charts/Data/Implementations/Standard/ChartDataSet.swift +++ b/Source/Charts/Data/Implementations/Standard/ChartDataSet.swift @@ -196,10 +196,11 @@ open class ChartDataSet: ChartBaseDataSet /// An empty array if no Entry object at that index. open override func entriesForXValue(_ xValue: Double) -> [ChartDataEntry] { - let match: (ChartDataEntry) -> Bool = { $0.x == xValue } + let match: (ChartDataEntry) -> Bool = { $0.x >= xValue } let i = partitioningIndex(where: match) - guard i < endIndex else { return [] } - return self[i...].prefix(while: match) + + guard self[i].x == xValue else { return [] } + return self[i...].prefix(while: { $0.x == xValue }) } /// - Parameters: @@ -234,6 +235,18 @@ open class ChartDataSet: ChartBaseDataSet } case .closest: + + let closestXIndex = closest + + formIndex(before: &closest) + let value = self[closest] + + // If the x value is closer to the original x index revert closest otherwise fall through + if abs(value.x - xValue) > abs(closestXValue - xValue) + { + closest = closestXIndex + } + break } diff --git a/Tests/ChartsTests/ChartDataTests.swift b/Tests/ChartsTests/ChartDataTests.swift index 5b697f710b..a7fa6f2bef 100644 --- a/Tests/ChartsTests/ChartDataTests.swift +++ b/Tests/ChartsTests/ChartDataTests.swift @@ -64,4 +64,43 @@ class ChartDataTests: XCTestCase { XCTAssertTrue(data.dataSet(forLabel: SetLabels.badLabel, ignorecase: true) == nil) XCTAssertTrue(data.dataSet(forLabel: SetLabels.badLabel, ignorecase: false) == nil) } + + func testEntriesForXValue() { + let entryCount = 38 + let startX = Double(1621858800.0) + let entries = (0 ..< entryCount).map { (i) -> ChartDataEntry in + let val = Double.random(in: 70...73) + return ChartDataEntry(x: startX+Double(i)*60.0, y: val) + } + + let set = ChartDataSet(entries: entries) + let slowMatch = set.firstIndex { $0.x == Double(1621860300)} + + let test1 = entries.partitioningIndex { $0.x >= Double(1621860300) } + + XCTAssertTrue(test1 == slowMatch) + + let test2 = entries.partitioningIndex { $0.x == Double(1621860300) } + + //this will fail since an exact match would rely on a 'mid' value in the partition algo to be a matching value. + //partitioningIndex(partitioningPoint) is not the same as binary search and should noot be used with exact matching criteria as it will not give a reliable result. + XCTAssertTrue(test1 != slowMatch) + + let res = set.entriesForXValue(Double(1621860300)) + let res2 = set.entriesForXValue(Double(1621860310)) + + XCTAssertTrue(test1 == slowMatch) + + let closestIdx = set.entryIndex(x: Double(1621860310), closestToY: .nan, rounding: .closest) + + XCTAssertTrue(closestIdx == slowMatch) + + let closestIdx2 = set.entryIndex(x: Double(1621860350), closestToY: .nan, rounding: .closest) + + XCTAssertTrue(closestIdx2 == slowMatch?.advanced(by: 1)) + + let closestIdx3 = set.entryIndex(x: Double(1621860330), closestToY: .nan, rounding: .closest) + + XCTAssertTrue(closestIdx3 == slowMatch) + } }