Charts: Should valueFormatter property use weak modifier?

[Huang-Libo]

Tihs property is found in Charts-Swift.h file, it's a delegate, should it use weak modifier to avoid retain cycle?

@property (nonatomic, strong) id <IChartAxisValueFormatter> _Nullable valueFormatter;

[pmairoldi]

Would probably make sense.

[Huang-Libo]

@petester42
I found a same issue: https://github.com/danielgindi/Charts/issues/1854

There are three ways to solve this problem:

1. Use weak to modify valueFormatter (I think it's better because valueFormatter is a delegate)
2. Use weakSelf to avoid retain cycle
3. Manually set valueFormatter to nil when leave the controller.

[liuxuan30]

Where do you see it? In which class?

[kientux]

@liuxuan30 It's in ChartAxisBase class. I had this problem too. I think if valueFormatter doesn't need to be strong property, it should be weak to prevent retain circle when assign it to self. It took me 2 hours to find out why my view controller didn't deallocated :(

[liuxuan30]

I filed a PR for related issues.

[liuxuan30]

I tested it today and found serious issues. If we apply weak, ChartsDemo won't display axis labels at all. Still investigating.

[liuxuan30]

Some findings:

1. if I change it to weak, _axisValueFormatter can't be correctly initialized. The getter of valueFormatter will even return nil, though we force to create a default one, however it does not set _axisValueFormatter.

1. changing to weak in swift code, will not impact the objective-C header file, it's still strong:
   >@property (nonatomic, strong) id _Nullable valueFormatter;

You might want to check on your side to see if it's true. I use latest Xcode 8.2.1. Although it's strong, but the behavior indeed has changed to a weak fashion, causing typical issues, like assigning a local variable will be dealloc after the function ends.

Sitting down and think of this, I don't think it's a good practice to use weak regardless the problems I had above, but if we really do so, people need to retain the formatter somewhere. So using weakSelf might be a better solution here.

[Huang-Libo]

@liuxuan30
Interesting...

[931743010]

use weakSelf ；Still unable to release
__weak typeof(self) weakSelf=self;
ChartXAxis *xl = _chartView.xAxis;
xl.valueFormatter = weakSelf;

[liuxuan30]

@931743010 just saying 'unable to release' can't help as from the code snippet, we can't say what's wrong. We don't have your project and even don't know if you retained self anywhere else. So if you want to prove it, make a simple project to reproduce.

[aelam]

seems I've the same problem

Sent from my iPhone

On 17 Mar 2017, at 2:41 PM, Xuan [e-mail] wrote:

@931743010 just saying 'unable to release' can't help as from the code snippet, we can't say what's wrong. We don't have your project and even don't know if you retained self anywhere else. So if you want to prove it, make a simple project to reproduce.

―
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[liuxuan30]

if weakSelf indeed don't work, try use a separate class as the formatter.