Skip to content

Commit

Permalink
Fixes 4641 & 4645
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
leeicmobile committed May 25, 2021
1 parent e91ba71 commit 8e52ea3
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 3 deletions.
19 changes: 16 additions & 3 deletions Source/Charts/Data/Implementations/Standard/ChartDataSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
}

Expand Down
39 changes: 39 additions & 0 deletions Tests/ChartsTests/ChartDataTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 8e52ea3

Please sign in to comment.