Added Safe index subscript logic for ChartData

[tringbhagya]

Issue Link 🔗

Crash Issue

• We face a crash when we attempt to highlight the values for the combined chart.

Implementation Details 🚧

• Added safe subscript logic.

[liuxuan30]

what's the effect of this PR? just return nil?

I would think adding such will only hide the issue and harder to find and make a fix for calculating highlight bounds correctly. Crashes always mean there is a defect especially OOB. Getting a real fix sometimes is better than hiding it. If there is no crash, then it's harder to notice then highlight is broken or mis-displaying in the field without any notice.

But I'm open to add this.

[tringbhagya]

Hi @liuxuan30, thank you for your response.

Actually this crash is happening when we highlight points in combined chart. For datasets, we are maintaining separate array. So, each chart renderer will take its corresponding data set value. But for highlight points, we have general array not specific to data set.

And also the chart data didn't extend MutableCollection in older version of this charts library and the dataset is taken safely by using this open func dataSet(at index: Index) -> Element? method. This mutablecollection's subscript method should return non-optional value. This is why i introduced new safe subscript method.

[liuxuan30]

we are working on release 5.0 to fix a bunch of compile errors and renaming the library. Probbaly have to revisit this PR after it's merged, or, you can target the release/5.0.0 branch and let @pmairoldi carry it in

[tringbhagya]

@liuxuan30 Thank you for the response. I'll check release/5.0.0 branch and include my changes.

[tringbhagya]

Hi @danielgindi @liuxuan30 @pmairoldi
It seems this crash is still occurring with latest release code. I've created a new pull request with latest release code. Please check it.
#5200