Replaced backing variables with proper access control #3104
Conversation
This is a non-breaking change No logic was changed, and the external interface remains the same. Only internal names have been changed (dropping "_") This excludes Bool values, as to change those breaks the external interface. Greatly reduces the code base, and keeps logic together.
|
@liuxuan30 Could you make this one a priority? It will cause merge conflicts in many of my PRs and I would like to get them out of the way. There are no logic changes, just removing backing variables across the board (except for Bools as noted in PR notes) like you've seen in a few other PRs already. |
|
@liuxuan30 I'm investigating why the test are failing. I'll let you know when it's fixed. |
|
Sorry just saw this. #3034 and this, which one first? |
|
#3034. Sounds good. I won't have time to fix this PR until the weekend. |
|
It's the end of year, so everyone is quite occupied :) I will try review 1-2 PR a week 😂 |
| /// The minimum interval between axis values. | ||
| /// This can be used to avoid label duplicating when zooming in. | ||
| /// | ||
| /// **default**: 1.0 | ||
| @objc open var granularity: Double | ||
| @objc open private(set) var granularity = 1.0 |
There was a problem hiding this comment.
old granularity setter is public.
| /// Use `resetCustomAxisMin()` to undo this. | ||
| @objc open internal(set) var axisMinimum = 0.0 { | ||
| willSet { | ||
| _customAxisMax = true |
There was a problem hiding this comment.
why willSet here? can't be in didSet?
There was a problem hiding this comment.
I was following the order in the former setters because I didn't want to change any logic in this PR. I agree it can all be done in didSet. Should I change it?
There was a problem hiding this comment.
I think so. The old setter is atomic I assume, so will/did seems the same in our case.
| /// The minimum value for this axis. | ||
| /// If set, this value will not be calculated automatically depending on the provided data. | ||
| /// Use `resetCustomAxisMin()` to undo this. | ||
| @objc open internal(set) var axisMinimum = 0.0 { |
There was a problem hiding this comment.
axisMinimum is also accessed from outside of framework.
| /// The maximum value for this axis. | ||
| /// If set, this value will not be calculated automatically depending on the provided data. | ||
| /// Use `resetCustomAxisMax()` to undo this. | ||
| @objc open internal(set) var axisMaximum = 0.0 { |
There was a problem hiding this comment.
Same to axisMinimum. Seems you missed some context to make setter internal?
There was a problem hiding this comment.
I think I was just tired and missed a couple things ; )
There was a problem hiding this comment.
Using Demo to test could help trigger the errors?
|
This is too big of a change to switch from master to 4.0.0. Will implement this in pieces as part of other changes. |
This is a non-breaking change
No logic was changed, and the external interface remains the same. Only internal names have been changed (dropping "_")
This excludes Bool values, as to change those breaks the external interface. Those changes are handled by #2687
Greatly reduces the code base, and keeps logic together.