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

Fix fatal error with x values in reverse order #4027

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jimmythai
Copy link

@jimmythai jimmythai commented Jun 16, 2019

Issue Link 🔗

Goals ⚽

Fixing the issues above and bugs as follows. Those bugs happen on BarChartViewController when you set entries which have x values in reverse order. A fatal error occurs on BubbleChartViewController, CandleStickChartViewController, LineChart1ViewController and ScatterChartViewController when you do the same things.

Implementation Details 🚧

Arrays of ChartDataEntry should be sorted in an ascending order when you initialize ChartDataSet and whenever you set them again, for instance, using replaceEntries(_:).

Testing Details 🔍

When you initialize ChartDataSet with entries that have x values in reverse order and Renderers execute draw methods, it causes Fatal error: Can't form Range with upperBound < lowerBound.
@codecov-io
Copy link

codecov-io commented Jun 16, 2019

Codecov Report

Merging #4027 into master will decrease coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4027      +/-   ##
==========================================
- Coverage   40.14%   40.13%   -0.01%     
==========================================
  Files         119      119              
  Lines       13985    13987       +2     
==========================================
  Hits         5614     5614              
- Misses       8371     8373       +2
Impacted Files Coverage Δ
...s/Data/Implementations/Standard/ChartDataSet.swift 39.51% <33.33%> (-0.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e850593...68a330c. Read the comment docs.

@liuxuan30
Copy link
Member

I wonder if it's our job to sort the array, or let the user handle it? @jjatie @petester42

@xymtek
Copy link
Contributor

xymtek commented Nov 15, 2019

sorting every-time the data is updated seems expensive. Maybe having some validation when drawing the next x should check if its larger than the previous x would be better.

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 this pull request may close these issues.

4 participants