Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor cleanup to Highlighter types #3003

Merged
merged 5 commits into from
Jan 6, 2018
Merged

Minor cleanup to Highlighter types #3003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c28880a
Minor cleanup to Highlighter types
jjatie Nov 14, 2017
df3b852
Fixes for PR
jjatie Nov 15, 2017
e66ba14
Merge remote-tracking branch 'upstream/master' into highlight-logic
jjatie Nov 15, 2017
796cb2d
Merge remote-tracking branch 'upstream/master' into highlight-logic
jjatie Jan 6, 2018
fa1297a
Pulled master and updated code style
jjatie Jan 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 34 additions & 44 deletions Source/Charts/Highlight/BarHighlighter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,25 @@ open class BarHighlighter: ChartHighlighter
{
open override func getHighlight(x: CGFloat, y: CGFloat) -> Highlight?
{
let high = super.getHighlight(x: x, y: y)
guard
let barData = (self.chart as? BarChartDataProvider)?.barData,
let high = super.getHighlight(x: x, y: y)
else { return nil }

if high == nil
let pos = getValsForTouch(x: x, y: y)

if let set = barData.getDataSetByIndex(high.dataSetIndex) as? IBarChartDataSet,
set.isStacked
{
return nil
return getStackedHighlight(high: high,
set: set,
xValue: Double(pos.x),
yValue: Double(pos.y))
}

if let barData = (self.chart as? BarChartDataProvider)?.barData
else
{
let pos = getValsForTouch(x: x, y: y)

if
let set = barData.getDataSetByIndex(high!.dataSetIndex) as? IBarChartDataSet,
set.isStacked
{
return getStackedHighlight(high: high!,
set: set,
xValue: Double(pos.x),
yValue: Double(pos.y))
}

return high
}
return nil
}

internal override func getDistance(x1: CGFloat, y1: CGFloat, x2: CGFloat, y2: CGFloat) -> CGFloat
Expand Down Expand Up @@ -75,25 +70,23 @@ open class BarHighlighter: ChartHighlighter
return high
}

if let ranges = entry.ranges,
guard
let ranges = entry.ranges,
ranges.count > 0
{
let stackIndex = getClosestStackIndex(ranges: ranges, value: yValue)

let pixel = chart
.getTransformer(forAxis: set.axisDependency)
.pixelForValues(x: high.x, y: ranges[stackIndex].to)

return Highlight(x: entry.x,
y: entry.y,
xPx: pixel.x,
yPx: pixel.y,
dataSetIndex: high.dataSetIndex,
stackIndex: stackIndex,
axis: high.axis)
}

return nil
else { return nil }

let stackIndex = getClosestStackIndex(ranges: ranges, value: yValue)
let pixel = chart
.getTransformer(forAxis: set.axisDependency)
.pixelForValues(x: high.x, y: ranges[stackIndex].to)

return Highlight(x: entry.x,
y: entry.y,
xPx: pixel.x,
yPx: pixel.y,
dataSetIndex: high.dataSetIndex,
stackIndex: stackIndex,
axis: high.axis)
}

/// - returns: The index of the closest value inside the values array / ranges (stacked barchart) to the value given as a parameter.
Expand All @@ -102,14 +95,11 @@ open class BarHighlighter: ChartHighlighter
/// - returns:
@objc open func getClosestStackIndex(ranges: [Range]?, value: Double) -> Int
{
if ranges == nil
{
return 0
}

guard let ranges = ranges else { return 0 }

var stackIndex = 0

for range in ranges!
for range in ranges
{
if range.contains(value)
{
Expand All @@ -121,8 +111,8 @@ open class BarHighlighter: ChartHighlighter
}
}

let length = max(ranges!.count - 1, 0)
let length = max(ranges.count - 1, 0)

return (value > ranges![length].to) ? length : 0
return (value > ranges[length].to) ? length : 0
}
}
61 changes: 21 additions & 40 deletions Source/Charts/Highlight/ChartHighlighter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ open class ChartHighlighter : NSObject, IHighlighter
open func getHighlight(x: CGFloat, y: CGFloat) -> Highlight?
{
let xVal = Double(getValsForTouch(x: x, y: y).x)

return getHighlight(xValue: xVal, x: x, y: y)
}

Expand All @@ -34,11 +33,10 @@ open class ChartHighlighter : NSObject, IHighlighter
/// - returns:
@objc open func getValsForTouch(x: CGFloat, y: CGFloat) -> CGPoint
{
guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider
else { return CGPoint.zero }
guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider else { return .zero }

// take any transformer to determine the values
return chart.getTransformer(forAxis: YAxis.AxisDependency.left).valueForTouchPoint(x: x, y: y)
return chart.getTransformer(forAxis: .left).valueForTouchPoint(x: x, y: y)
}

/// - returns: The corresponding ChartHighlight for a given x-value and xy-touch position in pixels.
Expand All @@ -48,19 +46,15 @@ open class ChartHighlighter : NSObject, IHighlighter
/// - returns:
@objc open func getHighlight(xValue xVal: Double, x: CGFloat, y: CGFloat) -> Highlight?
{
guard let chart = chart
else { return nil }
guard let chart = chart else { return nil }

let closestValues = getHighlights(xValue: xVal, x: x, y: y)
if closestValues.isEmpty
{
return nil
}
guard !closestValues.isEmpty else { return nil }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard is harder to read than how it was before. I have to think about it for a second when I try to understand this line. It was more straightforward before. It doesn't reduce nesting so we can switch it back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While guard often does reduce nesting, it's purpose is to indicate that

You use a guard statement to require that a condition must be true in order for the code after the guard statement to be executed. Apple.

It seems reasonable to me to expect reading "only continue if closestValues is not empty"


let leftAxisMinDist = getMinimumDistance(closestValues: closestValues, y: y, axis: YAxis.AxisDependency.left)
let rightAxisMinDist = getMinimumDistance(closestValues: closestValues, y: y, axis: YAxis.AxisDependency.right)
let leftAxisMinDist = getMinimumDistance(closestValues: closestValues, y: y, axis: .left)
let rightAxisMinDist = getMinimumDistance(closestValues: closestValues, y: y, axis: .right)

let axis = leftAxisMinDist < rightAxisMinDist ? YAxis.AxisDependency.left : YAxis.AxisDependency.right
let axis: YAxis.AxisDependency = leftAxisMinDist < rightAxisMinDist ? .left : .right

let detail = closestSelectionDetailByPixel(closestValues: closestValues, x: x, y: y, axis: axis, minSelectionDistance: chart.maxHighlightDistance)

Expand All @@ -77,24 +71,18 @@ open class ChartHighlighter : NSObject, IHighlighter
{
var vals = [Highlight]()

guard let
data = self.data
else { return vals }
guard let data = self.data else { return vals }

for i in 0 ..< data.dataSetCount
{
guard let dataSet = data.getDataSetByIndex(i)
guard
let dataSet = data.getDataSetByIndex(i),
dataSet.isHighlightEnabled // don't include datasets that cannot be highlighted
else { continue }

// don't include datasets that cannot be highlighted
if !dataSet.isHighlightEnabled
{
continue
}


// extract all y-values from all DataSets at the given x-value.
// some datasets (i.e bubble charts) make sense to have multiple values for an x-value. We'll have to find a way to handle that later on. It's more complicated now when x-indices are floating point.

vals.append(contentsOf: buildHighlights(dataSet: dataSet, dataSetIndex: i, xValue: xValue, rounding: .closest))
}

Expand All @@ -110,24 +98,21 @@ open class ChartHighlighter : NSObject, IHighlighter
{
var highlights = [Highlight]()

guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider
else { return highlights }
guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider else { return highlights }

var entries = set.entriesForXValue(xValue)
if entries.count == 0
if entries.count == 0, let closest = set.entryForXValue(xValue, closestToY: .nan, rounding: rounding)
{
// Try to find closest x-value and take all entries for that x-value
if let closest = set.entryForXValue(xValue, closestToY: Double.nan, rounding: rounding)
{
entries = set.entriesForXValue(closest.x)
}
entries = set.entriesForXValue(closest.x)
}

for e in entries
{
let px = chart.getTransformer(forAxis: set.axisDependency).pixelForValues(x: e.x, y: e.y)

highlights.append(Highlight(x: e.x, y: e.y, xPx: px.x, yPx: px.y, dataSetIndex: dataSetIndex, axis: set.axisDependency))

let highlight = Highlight(x: e.x, y: e.y, xPx: px.x, yPx: px.y, dataSetIndex: dataSetIndex, axis: set.axisDependency)
highlights.append(highlight)
}

return highlights
Expand All @@ -146,14 +131,12 @@ open class ChartHighlighter : NSObject, IHighlighter
var distance = minSelectionDistance
var closest: Highlight?

for i in 0 ..< closestValues.count
for high in closestValues
{
let high = closestValues[i]

if axis == nil || high.axis == axis
{
let cDistance = getDistance(x1: x, y1: y, x2: high.xPx, y2: high.yPx)

if cDistance < distance
{
closest = high
Expand All @@ -173,10 +156,8 @@ open class ChartHighlighter : NSObject, IHighlighter
{
var distance = CGFloat.greatestFiniteMagnitude

for i in 0 ..< closestValues.count
for high in closestValues
{
let high = closestValues[i]

if high.axis == axis
{
let tempDistance = abs(getHighlightPos(high: high) - y)
Expand Down
52 changes: 22 additions & 30 deletions Source/Charts/Highlight/CombinedHighlighter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,46 +30,38 @@ open class CombinedHighlighter: ChartHighlighter
{
var vals = [Highlight]()

guard let chart = self.chart as? CombinedChartDataProvider
guard
let chart = self.chart as? CombinedChartDataProvider,
let dataObjects = chart.combinedData?.allData
else { return vals }

if let dataObjects = chart.combinedData?.allData
for i in 0..<dataObjects.count
{
for i in 0..<dataObjects.count
let dataObject = dataObjects[i]

// in case of BarData, let the BarHighlighter take over
if barHighlighter != nil && dataObject is BarChartData,
let high = barHighlighter?.getHighlight(x: x, y: y)
{
high.dataIndex = i
vals.append(high)
}
else
{
let dataObject = dataObjects[i]

// in case of BarData, let the BarHighlighter take over
if barHighlighter != nil && dataObject is BarChartData
for j in 0..<dataObject.dataSetCount
{
if let high = barHighlighter?.getHighlight(x: x, y: y)
guard let dataSet = dataObject.getDataSetByIndex(j),
dataSet.isHighlightEnabled // don't include datasets that cannot be highlighted
else { continue }

let highs = buildHighlights(dataSet: dataSet, dataSetIndex: j, xValue: xValue, rounding: .closest)

for high in highs
{
high.dataIndex = i
vals.append(high)
}
}
else
{
for j in 0..<dataObject.dataSetCount
{
guard let dataSet = dataObjects[i].getDataSetByIndex(j)
else { continue }

// don't include datasets that cannot be highlighted
if !dataSet.isHighlightEnabled
{
continue
}

let highs = buildHighlights(dataSet: dataSet, dataSetIndex: j, xValue: xValue, rounding: .closest)

for high in highs
{
high.dataIndex = i
vals.append(high)
}
}
}
}
}

Expand Down
1 change: 0 additions & 1 deletion Source/Charts/Highlight/Highlight.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,5 @@ extension Highlight /*: Equatable*/ {
&& dataIndex == object.dataIndex
&& _dataSetIndex == object._dataSetIndex
&& _stackIndex == object._stackIndex

}
}
40 changes: 16 additions & 24 deletions Source/Charts/Highlight/HorizontalBarHighlighter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,21 @@ open class HorizontalBarHighlighter: BarHighlighter
{
open override func getHighlight(x: CGFloat, y: CGFloat) -> Highlight?
{
if let barData = self.chart?.data as? BarChartData
guard let barData = self.chart?.data as? BarChartData else { return nil }

let pos = getValsForTouch(x: y, y: x)
guard let high = getHighlight(xValue: Double(pos.y), x: y, y: x) else { return nil }

if let set = barData.getDataSetByIndex(high.dataSetIndex) as? IBarChartDataSet,
set.isStacked
{
let pos = getValsForTouch(x: y, y: x)

guard let high = getHighlight(xValue: Double(pos.y), x: y, y: x)
else { return nil }

if let set = barData.getDataSetByIndex(high.dataSetIndex) as? IBarChartDataSet,
set.isStacked
{
return getStackedHighlight(high: high,
set: set,
xValue: Double(pos.y),
yValue: Double(pos.x))
}

return high
return getStackedHighlight(high: high,
set: set,
xValue: Double(pos.y),
yValue: Double(pos.x))
}
return nil

return high
}

internal override func buildHighlights(
Expand All @@ -46,17 +42,13 @@ open class HorizontalBarHighlighter: BarHighlighter
{
var highlights = [Highlight]()

guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider
else { return highlights }
guard let chart = self.chart as? BarLineScatterCandleBubbleChartDataProvider else { return highlights }

var entries = set.entriesForXValue(xValue)
if entries.count == 0
if entries.count == 0, let closest = set.entryForXValue(xValue, closestToY: .nan, rounding: rounding)
{
// Try to find closest x-value and take all entries for that x-value
if let closest = set.entryForXValue(xValue, closestToY: Double.nan, rounding: rounding)
{
entries = set.entriesForXValue(closest.x)
}
entries = set.entriesForXValue(closest.x)
}

for e in entries
Expand Down
Loading