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

Jump to bottom

Removed redundant ivars in BarLineChartViewBase #3043

Merged
merged 4 commits into from
Dec 25, 2017
Merged

Removed redundant ivars in BarLineChartViewBase #3043

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94a49b5
Removed redundant ivars
jjatie Nov 23, 2017
545e1f6
Merge remote-tracking branch 'upstream/master' into barlinechartviewb…
jjatie Dec 5, 2017
6e8c296
Merge remote-tracking branch 'upstream/master' into barlinechartviewb…
jjatie Dec 25, 2017
55626bb
Moved initialization of axes to their declaration to keep the same op…
jjatie Dec 25, 2017
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
4 changes: 2 additions & 2 deletions Source/Charts/Charts/BarChartView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ open class BarChartView: BarLineChartViewBase, BarChartDataProvider
}

// calculate axis range (min / max) according to provided data
_leftAxis.calculate(
leftAxis.calculate(
min: data.getYMin(axis: .left),
max: data.getYMax(axis: .left))
_rightAxis.calculate(
rightAxis.calculate(
min: data.getYMin(axis: .right),
max: data.getYMax(axis: .right))
}
Expand Down
171 changes: 71 additions & 100 deletions Source/Charts/Charts/BarLineChartViewBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,28 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
/// **default**: false
@objc open var keepPositionOnRotation: Bool = false

/// the object representing the left y-axis
internal var _leftAxis: YAxis!
/// The left y-axis object. In the horizontal bar-chart, this is the
/// top axis.
@objc open internal(set) var leftAxis: YAxis!
Copy link
Member

Choose a reason for hiding this comment

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

what's the set purpose here?

Copy link
Collaborator Author

@jjatie jjatie Dec 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Previously there were two variables

internal var _leftAxis: YAxis!
@objc open var leftAxis: YAxis! {
    return _leftAxis
}

There is no need for this as Swift provides better access control than objective-c did previously. open internal(set) (can also be open private(set) but was holding off on that one until I evaluated whether that was appropriate) makes the variable open, but only allows it to be set by internally.

Copy link
Member

Choose a reason for hiding this comment

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

wow, I don't realize swift add this already.

Copy link
Member

Choose a reason for hiding this comment

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

Just a moment. Have you checked if people can set axis outside from the framework? Just like other PRs similar issue?

Copy link
Member

Choose a reason for hiding this comment

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

I just did a quick check, axis is not settable outside, seems safe. Just run the tests and demo:)


/// the object representing the right y-axis
internal var _rightAxis: YAxis!
/// The right y-axis object. In the horizontal bar-chart, this is the
/// bottom axis.
@objc open internal(set) var rightAxis: YAxis!

/// The left Y axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of YAxisRenderer
@objc open var leftYAxisRenderer: YAxisRenderer!

internal var _leftYAxisRenderer: YAxisRenderer!
internal var _rightYAxisRenderer: YAxisRenderer!
/// The right Y axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of YAxisRenderer
@objc open var rightYAxisRenderer: YAxisRenderer!

internal var _leftAxisTransformer: Transformer!
Copy link
Member

Choose a reason for hiding this comment

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

how about these two and some others with _? Like _tapGestureRecognizer It means we still need _ for some?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It means I didn't touch them. I think they shouldn't have the _.

internal var _rightAxisTransformer: Transformer!

internal var _xAxisRenderer: XAxisRenderer!
/// The X axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of XAxisRenderer
@objc open var xAxisRenderer: XAxisRenderer!

internal var _tapGestureRecognizer: NSUITapGestureRecognizer!
internal var _doubleTapGestureRecognizer: NSUITapGestureRecognizer!
Expand Down Expand Up @@ -100,16 +109,16 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
super.initialize()

_leftAxis = YAxis(position: .left)
_rightAxis = YAxis(position: .right)
leftAxis = YAxis(position: .left)
rightAxis = YAxis(position: .right)

_leftAxisTransformer = Transformer(viewPortHandler: _viewPortHandler)
_rightAxisTransformer = Transformer(viewPortHandler: _viewPortHandler)

_leftYAxisRenderer = YAxisRenderer(viewPortHandler: _viewPortHandler, yAxis: _leftAxis, transformer: _leftAxisTransformer)
_rightYAxisRenderer = YAxisRenderer(viewPortHandler: _viewPortHandler, yAxis: _rightAxis, transformer: _rightAxisTransformer)
leftYAxisRenderer = YAxisRenderer(viewPortHandler: _viewPortHandler, yAxis: leftAxis, transformer: _leftAxisTransformer)
rightYAxisRenderer = YAxisRenderer(viewPortHandler: _viewPortHandler, yAxis: rightAxis, transformer: _rightAxisTransformer)

_xAxisRenderer = XAxisRenderer(viewPortHandler: _viewPortHandler, xAxis: _xAxis, transformer: _leftAxisTransformer)
xAxisRenderer = XAxisRenderer(viewPortHandler: _viewPortHandler, xAxis: _xAxis, transformer: _leftAxisTransformer)

self.highlighter = ChartHighlighter(chart: self)

Expand Down Expand Up @@ -178,43 +187,43 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
autoScale()
}

if _leftAxis.isEnabled
if leftAxis.isEnabled
{
_leftYAxisRenderer?.computeAxis(min: _leftAxis._axisMinimum, max: _leftAxis._axisMaximum, inverted: _leftAxis.isInverted)
leftYAxisRenderer?.computeAxis(min: leftAxis._axisMinimum, max: leftAxis._axisMaximum, inverted: leftAxis.isInverted)
}

if _rightAxis.isEnabled
if rightAxis.isEnabled
{
_rightYAxisRenderer?.computeAxis(min: _rightAxis._axisMinimum, max: _rightAxis._axisMaximum, inverted: _rightAxis.isInverted)
rightYAxisRenderer?.computeAxis(min: rightAxis._axisMinimum, max: rightAxis._axisMaximum, inverted: rightAxis.isInverted)
}

if _xAxis.isEnabled
{
_xAxisRenderer?.computeAxis(min: _xAxis._axisMinimum, max: _xAxis._axisMaximum, inverted: false)
xAxisRenderer?.computeAxis(min: _xAxis._axisMinimum, max: _xAxis._axisMaximum, inverted: false)
}

_xAxisRenderer?.renderAxisLine(context: context)
_leftYAxisRenderer?.renderAxisLine(context: context)
_rightYAxisRenderer?.renderAxisLine(context: context)
xAxisRenderer?.renderAxisLine(context: context)
leftYAxisRenderer?.renderAxisLine(context: context)
rightYAxisRenderer?.renderAxisLine(context: context)

// The renderers are responsible for clipping, to account for line-width center etc.
_xAxisRenderer?.renderGridLines(context: context)
_leftYAxisRenderer?.renderGridLines(context: context)
_rightYAxisRenderer?.renderGridLines(context: context)
xAxisRenderer?.renderGridLines(context: context)
leftYAxisRenderer?.renderGridLines(context: context)
rightYAxisRenderer?.renderGridLines(context: context)

if _xAxis.isEnabled && _xAxis.isDrawLimitLinesBehindDataEnabled
{
_xAxisRenderer?.renderLimitLines(context: context)
xAxisRenderer?.renderLimitLines(context: context)
}

if _leftAxis.isEnabled && _leftAxis.isDrawLimitLinesBehindDataEnabled
if leftAxis.isEnabled && leftAxis.isDrawLimitLinesBehindDataEnabled
{
_leftYAxisRenderer?.renderLimitLines(context: context)
leftYAxisRenderer?.renderLimitLines(context: context)
}

if _rightAxis.isEnabled && _rightAxis.isDrawLimitLinesBehindDataEnabled
if rightAxis.isEnabled && rightAxis.isDrawLimitLinesBehindDataEnabled
{
_rightYAxisRenderer?.renderLimitLines(context: context)
rightYAxisRenderer?.renderLimitLines(context: context)
}

// make sure the data cannot be drawn outside the content-rect
Expand All @@ -234,22 +243,22 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD

if _xAxis.isEnabled && !_xAxis.isDrawLimitLinesBehindDataEnabled
{
_xAxisRenderer?.renderLimitLines(context: context)
xAxisRenderer?.renderLimitLines(context: context)
}

if _leftAxis.isEnabled && !_leftAxis.isDrawLimitLinesBehindDataEnabled
if leftAxis.isEnabled && !leftAxis.isDrawLimitLinesBehindDataEnabled
{
_leftYAxisRenderer?.renderLimitLines(context: context)
leftYAxisRenderer?.renderLimitLines(context: context)
}

if _rightAxis.isEnabled && !_rightAxis.isDrawLimitLinesBehindDataEnabled
if rightAxis.isEnabled && !rightAxis.isDrawLimitLinesBehindDataEnabled
{
_rightYAxisRenderer?.renderLimitLines(context: context)
rightYAxisRenderer?.renderLimitLines(context: context)
}

_xAxisRenderer.renderAxisLabels(context: context)
_leftYAxisRenderer.renderAxisLabels(context: context)
_rightYAxisRenderer.renderAxisLabels(context: context)
xAxisRenderer.renderAxisLabels(context: context)
leftYAxisRenderer.renderAxisLabels(context: context)
rightYAxisRenderer.renderAxisLabels(context: context)

if clipValuesToContentEnabled
{
Expand Down Expand Up @@ -287,29 +296,29 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD

// calculate axis range (min / max) according to provided data

if _leftAxis.isEnabled
if leftAxis.isEnabled
{
_leftAxis.calculate(min: data.getYMin(axis: .left), max: data.getYMax(axis: .left))
leftAxis.calculate(min: data.getYMin(axis: .left), max: data.getYMax(axis: .left))
}

if _rightAxis.isEnabled
if rightAxis.isEnabled
{
_rightAxis.calculate(min: data.getYMin(axis: .right), max: data.getYMax(axis: .right))
rightAxis.calculate(min: data.getYMin(axis: .right), max: data.getYMax(axis: .right))
}

calculateOffsets()
}

internal func prepareValuePxMatrix()
{
_rightAxisTransformer.prepareMatrixValuePx(chartXMin: _xAxis._axisMinimum, deltaX: CGFloat(xAxis.axisRange), deltaY: CGFloat(_rightAxis.axisRange), chartYMin: _rightAxis._axisMinimum)
_leftAxisTransformer.prepareMatrixValuePx(chartXMin: xAxis._axisMinimum, deltaX: CGFloat(xAxis.axisRange), deltaY: CGFloat(_leftAxis.axisRange), chartYMin: _leftAxis._axisMinimum)
_rightAxisTransformer.prepareMatrixValuePx(chartXMin: _xAxis._axisMinimum, deltaX: CGFloat(xAxis.axisRange), deltaY: CGFloat(rightAxis.axisRange), chartYMin: rightAxis._axisMinimum)
_leftAxisTransformer.prepareMatrixValuePx(chartXMin: xAxis._axisMinimum, deltaX: CGFloat(xAxis.axisRange), deltaY: CGFloat(leftAxis.axisRange), chartYMin: leftAxis._axisMinimum)
}

internal func prepareOffsetMatrix()
{
_rightAxisTransformer.prepareMatrixOffset(inverted: _rightAxis.isInverted)
_leftAxisTransformer.prepareMatrixOffset(inverted: _leftAxis.isInverted)
_rightAxisTransformer.prepareMatrixOffset(inverted: rightAxis.isInverted)
_leftAxisTransformer.prepareMatrixOffset(inverted: leftAxis.isInverted)
}

open override func notifyDataSetChanged()
Expand All @@ -318,12 +327,12 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD

calcMinMax()

_leftYAxisRenderer?.computeAxis(min: _leftAxis._axisMinimum, max: _leftAxis._axisMaximum, inverted: _leftAxis.isInverted)
_rightYAxisRenderer?.computeAxis(min: _rightAxis._axisMinimum, max: _rightAxis._axisMaximum, inverted: _rightAxis.isInverted)
leftYAxisRenderer?.computeAxis(min: leftAxis._axisMinimum, max: leftAxis._axisMaximum, inverted: leftAxis.isInverted)
rightYAxisRenderer?.computeAxis(min: rightAxis._axisMinimum, max: rightAxis._axisMaximum, inverted: rightAxis.isInverted)

if let data = _data
{
_xAxisRenderer?.computeAxis(
xAxisRenderer?.computeAxis(
min: _xAxis._axisMinimum,
max: _xAxis._axisMaximum,
inverted: false)
Expand All @@ -345,8 +354,8 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
_xAxis.calculate(min: _data?.xMin ?? 0.0, max: _data?.xMax ?? 0.0)

// calculate axis range (min / max) according to provided data
_leftAxis.calculate(min: _data?.getYMin(axis: .left) ?? 0.0, max: _data?.getYMax(axis: .left) ?? 0.0)
_rightAxis.calculate(min: _data?.getYMin(axis: .right) ?? 0.0, max: _data?.getYMax(axis: .right) ?? 0.0)
leftAxis.calculate(min: _data?.getYMin(axis: .left) ?? 0.0, max: _data?.getYMax(axis: .left) ?? 0.0)
rightAxis.calculate(min: _data?.getYMin(axis: .right) ?? 0.0, max: _data?.getYMax(axis: .right) ?? 0.0)
}

internal func calculateLegendOffsets(offsetLeft: inout CGFloat, offsetTop: inout CGFloat, offsetRight: inout CGFloat, offsetBottom: inout CGFloat)
Expand Down Expand Up @@ -1716,28 +1725,17 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
/// if the chart is fully zoomed out, return true
@objc open var isFullyZoomedOut: Bool { return _viewPortHandler.isFullyZoomedOut }

/// - returns: The left y-axis object. In the horizontal bar-chart, this is the
/// top axis.
@objc open var leftAxis: YAxis
{
return _leftAxis
}

/// - returns: The right y-axis object. In the horizontal bar-chart, this is the
/// bottom axis.
@objc open var rightAxis: YAxis { return _rightAxis }

/// - returns: The y-axis object to the corresponding AxisDependency. In the
/// horizontal bar-chart, LEFT == top, RIGHT == BOTTOM
@objc open func getAxis(_ axis: YAxis.AxisDependency) -> YAxis
{
if axis == .left
{
return _leftAxis
return leftAxis
}
else
{
return _rightAxis
return rightAxis
}
}

Expand Down Expand Up @@ -1781,33 +1779,6 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
/// - returns: `true` if both drag offsets (x and y) are zero or smaller.
@objc open var hasNoDragOffset: Bool { return _viewPortHandler.hasNoDragOffset }

/// The X axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of XAxisRenderer
/// - returns: The current set X axis renderer
@objc open var xAxisRenderer: XAxisRenderer
{
get { return _xAxisRenderer }
set { _xAxisRenderer = newValue }
}

/// The left Y axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of YAxisRenderer
/// - returns: The current set left Y axis renderer
@objc open var leftYAxisRenderer: YAxisRenderer
{
get { return _leftYAxisRenderer }
set { _leftYAxisRenderer = newValue }
}

/// The right Y axis renderer. This is a read-write property so you can set your own custom renderer here.
/// **default**: An instance of YAxisRenderer
/// - returns: The current set right Y axis renderer
@objc open var rightYAxisRenderer: YAxisRenderer
{
get { return _rightYAxisRenderer }
set { _rightYAxisRenderer = newValue }
}

open override var chartYMax: Double
{
return max(leftAxis._axisMaximum, rightAxis._axisMaximum)
Expand All @@ -1821,7 +1792,7 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
/// - returns: `true` if either the left or the right or both axes are inverted.
@objc open var isAnyAxisInverted: Bool
{
return _leftAxis.isInverted || _rightAxis.isInverted
return leftAxis.isInverted || rightAxis.isInverted
}

/// flag that indicates if auto scaling on the y axis is enabled.
Expand All @@ -1841,11 +1812,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
if axis == .left
{
_leftAxis.minWidth = width
leftAxis.minWidth = width
}
else
{
_rightAxis.minWidth = width
rightAxis.minWidth = width
}
}

Expand All @@ -1855,11 +1826,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
if axis == .left
{
return _leftAxis.minWidth
return leftAxis.minWidth
}
else
{
return _rightAxis.minWidth
return rightAxis.minWidth
}
}
/// Sets a maximum width to the specified y axis.
Expand All @@ -1868,11 +1839,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
if axis == .left
{
_leftAxis.maxWidth = width
leftAxis.maxWidth = width
}
else
{
_rightAxis.maxWidth = width
rightAxis.maxWidth = width
}
}

Expand All @@ -1884,11 +1855,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
if axis == .left
{
return _leftAxis.maxWidth
return leftAxis.maxWidth
}
else
{
return _rightAxis.maxWidth
return rightAxis.maxWidth
}
}

Expand All @@ -1897,11 +1868,11 @@ open class BarLineChartViewBase: ChartViewBase, BarLineScatterCandleBubbleChartD
{
if axis == .left
{
return _leftAxis.requiredSize().width
return leftAxis.requiredSize().width
}
else
{
return _rightAxis.requiredSize().width
return rightAxis.requiredSize().width
}
}

Expand Down
Loading