ChartsOrg / ChartsOrg/Charts

Collection operations like `removeAll` should not invalidate references

Open
#4,946 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Swift
Stars
28k
Forks
6k
PR merge metrics
No merged PRs in 30d

Description

* [x] I've read, understood, and done my best to follow the [*CONTRIBUTING guidelines](https://github.com/jjatie/Charts/blob/master/CONTRIBUTING.md).

## What did you do?

Call `removeAll` on a DataSet (specifically `LineChartDataSet`, although this applies to many subclasses):

var newSeries = oldSeries
newSeries.removeAll()
assert(newSeries == oldSeries)

## What did you expect to happen?

`removeAll` should remove entries in the data set, while keeping the instance the same.
The assertion should not fail.
In other words, it should not invalidate other references to the data set.

The Charts library has chosen to define data sets as classes (reference types). However,
the inherited methods from the collection protocols are not consistent with reference type semantics.

## What happened instead?

Calling `removeAll` creates a new data set (unless `keepingCapacity: true`). This makes sense for a value type, but it is quite confusing for a reference type.

This can be solved by overriding the default `removeAll` implementation
and doing it properly. For example, the internal `entries` array can be thrown away and recreated, without invalidating the entire ChartDataSet object.

## Charts Environment

**Charts version/Branch/Commit Number:** 4.1.0
**Xcode version:** 14.0.1
**Swift version:**
**Platform(s) running Charts:** iOS

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.