Skip to content

Commit

Permalink
Fixes an issue with string comparison in ChartData for finding a data…
Browse files Browse the repository at this point in the history
…set by its label (Fixes ChartsOrg#4274)

ChartData::getDataSetByLabel has a bug where the string comparison is checking the same string against itself due to the functions parameter having the same name as a variable within the closures scope. This change renames the variable to fix the issue.
  • Loading branch information
PeterKaminski09 committed Jan 24, 2020
1 parent 1bbec78 commit 66d50a8
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 4 deletions.
69 changes: 69 additions & 0 deletions ChartDataTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//
// ChartDataTests.swift
// ChartsTests
//
// Created by Peter Kaminski on 1/23/20.
//

import XCTest
@testable import Charts

class ChartDataTests: XCTestCase {

var data: ScatterChartData!

private enum SetLabels {
static let one = "label1"
static let two = "label2"
static let three = "label3"
static let badLabel = "Bad label"
}

override func setUp() {
super.setUp()

let setCount = 5
let range: UInt32 = 32
let values1 = (0..<setCount).map { (i) -> ChartDataEntry in
let val = Double(arc4random_uniform(range) + 3)
return ChartDataEntry(x: Double(i), y: val)
}
let values2 = (0..<setCount).map { (i) -> ChartDataEntry in
let val = Double(arc4random_uniform(range) + 3)
return ChartDataEntry(x: Double(i), y: val)
}
let values3 = (0..<setCount).map { (i) -> ChartDataEntry in
let val = Double(arc4random_uniform(range) + 3)
return ChartDataEntry(x: Double(i), y: val)
}

let set1 = ScatterChartDataSet(entries: values1, label: SetLabels.one)
let set2 = ScatterChartDataSet(entries: values2, label: SetLabels.two)
let set3 = ScatterChartDataSet(entries: values3, label: SetLabels.three)

data = ScatterChartData(dataSets: [set1, set2, set3])
}

func testGetDataSetByLabelCaseSensitive() {
XCTAssertTrue(data.getDataSetByLabel(SetLabels.one, ignorecase: false)?.label == SetLabels.one)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.two, ignorecase: false)?.label == SetLabels.two)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.three, ignorecase: false)?.label == SetLabels.three)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.one.uppercased(), ignorecase: false) == nil)
}

func testGetDataSetByLabelIgnoreCase() {
XCTAssertTrue(data.getDataSetByLabel(SetLabels.one, ignorecase: true)?.label == SetLabels.one)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.two, ignorecase: true)?.label == SetLabels.two)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.three, ignorecase: true)?.label == SetLabels.three)

XCTAssertTrue(data.getDataSetByLabel(SetLabels.one.uppercased(), ignorecase: true)?.label == SetLabels.one)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.two.uppercased(), ignorecase: true)?.label == SetLabels.two)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.three.uppercased(), ignorecase: true)?.label == SetLabels.three)
}

func testGetDataSetByLabelNilWithBadLabel() {
XCTAssertTrue(data.getDataSetByLabel(SetLabels.badLabel, ignorecase: true) == nil)
XCTAssertTrue(data.getDataSetByLabel(SetLabels.badLabel, ignorecase: false) == nil)
}
}

4 changes: 4 additions & 0 deletions Charts.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
B6DCC229615EFE706F64A37D /* LineScatterCandleRadarRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 923206233CA89FD03565FF87 /* LineScatterCandleRadarRenderer.swift */; };
B85DEB06B4C1AFFC8A0E3295 /* CircleShapeRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = ECE1B1623D3AF69CECAE8562 /* CircleShapeRenderer.swift */; };
BEFD9518F3A74ACF8FA33308 /* Charts.h in Headers */ = {isa = PBXBuildFile; fileRef = 4F9922F0641F7955DC6CD324 /* Charts.h */; settings = {ATTRIBUTES = (Public, ); }; };
C03E6D8123DAAB2600083010 /* ChartDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C03E6D8023DAAB2600083010 /* ChartDataTests.swift */; };
C04D269AD4A373FD2B621C43 /* LineChartData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C978F31F23C7D21197DC2A1 /* LineChartData.swift */; };
C09E91F67A4AC43C277E7D82 /* BubbleChartDataEntry.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD8ED233775EEC31243A6919 /* BubbleChartDataEntry.swift */; };
C20A62D8CB9120523D5FB650 /* LegendEntry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7C673B9ED4340F550A9283 /* LegendEntry.swift */; };
Expand Down Expand Up @@ -286,6 +287,7 @@
BD02157CF8CEE1189BF681DA /* PieChartDataEntry.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PieChartDataEntry.swift; path = Source/Charts/Data/Implementations/Standard/PieChartDataEntry.swift; sourceTree = "<group>"; };
BD5C6D20243EC2F19069AACD /* CandleStickChartRenderer.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = CandleStickChartRenderer.swift; path = Source/Charts/Renderers/CandleStickChartRenderer.swift; sourceTree = "<group>"; };
BFABD027DAF6851088F002AC /* LineChartDataProvider.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = LineChartDataProvider.swift; path = Source/Charts/Interfaces/LineChartDataProvider.swift; sourceTree = "<group>"; };
C03E6D8023DAAB2600083010 /* ChartDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChartDataTests.swift; sourceTree = "<group>"; };
C31AA65EA27776F8C653C7E8 /* BarChartDataSet.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = BarChartDataSet.swift; path = Source/Charts/Data/Implementations/Standard/BarChartDataSet.swift; sourceTree = "<group>"; };
C52E8344160B5E689DA3C25C /* ChevronDownShapeRenderer.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = ChevronDownShapeRenderer.swift; path = Source/Charts/Renderers/Scatter/ChevronDownShapeRenderer.swift; sourceTree = "<group>"; };
C574E1BC7E12D937A5471EF8 /* Info.plist */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.plist.xml; name = Info.plist; path = "Tests/Supporting Files/Info.plist"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -537,6 +539,7 @@
D2E1819D72CD7B6C4A4E8048 /* LineChartTests.swift */,
135F11CD20425AF600D655A3 /* PieChartTests.swift */,
064989451F5C99C7006E8BB3 /* Snapshot.swift */,
C03E6D8023DAAB2600083010 /* ChartDataTests.swift */,
);
name = Charts;
sourceTree = "<group>";
Expand Down Expand Up @@ -985,6 +988,7 @@
B66817462241E3CC00017CF1 /* HorizontalBarChartTests.swift in Sources */,
135F11CE20425AF600D655A3 /* PieChartTests.swift in Sources */,
064989461F5C99C7006E8BB3 /* Snapshot.swift in Sources */,
C03E6D8123DAAB2600083010 /* ChartDataTests.swift in Sources */,
224EFF991FBAAC4700CF9B3B /* EquatableTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
6 changes: 2 additions & 4 deletions Source/Charts/Data/Implementations/Standard/ChartData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,8 @@ open class ChartData: NSObject
// TODO: Return nil instead of -1
if ignorecase
{
return dataSets.firstIndex {
guard let label = $0.label else { return false }
return label.caseInsensitiveCompare(label) == .orderedSame
} ?? -1
return dataSets.firstIndex { $0.label?.caseInsensitiveCompare(label) == .orderedSame }
?? -1
}
else
{
Expand Down

0 comments on commit 66d50a8

Please sign in to comment.