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.

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

ChartData::getDataSetByLabel always return the first IChartDataSet when ignorecase = true #4274

Closed
1 task done
PeterKaminski09 opened this issue Jan 23, 2020 · 0 comments · Fixed by #4275
Closed
1 task done

Comments

@PeterKaminski09
Copy link
Contributor

PeterKaminski09 commented Jan 23, 2020

Issue

It appears that this commit (https://github.com/danielgindi/Charts/pull/3892/files) added the comparison label.caseInsensitiveCompare(label) to ChartData::getDataSetByLabel.

This will always be true, so statements like chartData.getDataSetByLabel("myLabel", ignorecase: true) will return whatever IChartDataSet is at index 0.

Effected code

 internal func getDataSetIndexByLabel(_ label: String, ignorecase: Bool) -> Int
    {
        // 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
        }
        ...
    }

Follow-up

I'm happy to submit a pull-request to fix this issue 😺

Charts Environment

Charts version/Branch/Commit Number: 3.4.0
Xcode version: 11.3.1
Swift version: 5.0
Platform(s) running Charts: iOS
macOS version running Xcode: 10.15.2

@PeterKaminski09 PeterKaminski09 changed the title ChartData::getDataSetByLabel always return true when ignorecase = true ChartData::getDataSetByLabel always return the first IChartDataSet when ignorecase = true Jan 23, 2020
PeterKaminski09 added a commit to PeterKaminski09/Charts that referenced this issue Jan 24, 2020
…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.
danielgindi added a commit that referenced this issue Jan 24, 2020
Fix for #4274 string comparison issue in ChartData::getDataSetByLabel
water-su added a commit to water-su/Charts that referenced this issue Feb 7, 2020
* origin/master: (256 commits)
  Add our first super sponsor!
  Updated test screenshots
  Add option to draw grid lines in front of data
  Removed double spacing for labelRotatedHeight
  Fixes an issue with string comparison in ChartData for finding a dataset by its label (Fixes ChartsOrg#4274)
  Simplify code
  Use faster check for line whether it's inside drawing rect
  Corrected pie `isHighlightEnabled` - render normally when disabled
  Restored correct velocity sampler
  Deduplicate BalloonMarker code
  Unified styling
  Call chartViewDidEndPanning on when *panning* is ended
  Propagate file header to newer files
  Removed redundant minEntries > 0
  labelXOffset = 10 is default for radar chart only
  Update README.md, added link to tutorial about Radar Charts.
  Changes to fix Catalyst compatibility
  Allowing overrides for YAxisRenderer.drawYLabels (ChartsOrg#4089)
  move isDrawCirclesEnabled check further up in code to avoid creating … (ChartsOrg#4050)
  Platform separation (ChartsOrg#4178)
  ...

# Conflicts:
#	Source/Charts/Components/AxisBase.swift
#	Source/Charts/Data/Implementations/Standard/LineRadarChartDataSet.swift
#	Source/Charts/Highlight/Highlight.swift
SwiftPolar pushed a commit to SwiftPolar/Charts that referenced this issue Mar 20, 2023
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant