Added a safety check before an unsafe array operation

[UberNick]

Issue Link 🔗

#4001

Goals ⚽

This code change helps safeguard against hard crashes caused by an array operation. It doesn't fix potential issues that could cause such a condition in the first place.

Testing Details 🔍

I manually stepped through the attached demo project and verified the PieChart views looked and behaved as expected.

[liuxuan30]

A) I checked the code, it seems accessibleChartElements will have at least one element in drawDataSet normally, unless drawDataSet exits early, such as

    @objc open func drawDataSet(context: CGContext, dataSet: IPieChartDataSet)
    {
        guard let chart = chart else {return }

otherwise it should have at least 1 default element and insert(:_, at: 1) won't crash then. I don't think a low memory case would cause the crash here, if it's really true, then I think it's Swift bug rather than ours.

Now since you said it's crashed, I suppose above guard failed probably. Can you double check?

B) The safe check seems harmless to me, and I'm ok to merge here. But we need to discuss a little more before we merge.

[UberNick]

Thanks for looking into this, @liuxuan30. I agree with your assessment that this condition shouldn't happen through normal use, and the rarity of the error among our user base combined with my inability to reproduce the problem reinforces that conclusion.

In my use case, Charts elements are being used within recycled TableViewCells and being driven by asynchronous data requests. Perhaps the error only occurs due to a rare race-condition or thread-related problem. One that may be more likely, perhaps even necessary, on devices where asynchronous processes are being suspended or terminated due to out-of-memory issues.

This is total speculation from me, and I don't have data to support the above statement. In the absence of other data, though, I think it's a good guess that's reasonably addressed by this otherwise innocuous PR.