从一段奇葩的objc代码看代码规范的重要性

背景介绍

昨天,在和我一个朋友讨论,到底是用self.propertyName还是_propertyName来访问property,我认为应该使用self.propertyName,因为我在听Stanford Open Course的时候,苹果的工程师告诫要使用self.propertyName,不要使用_propertyName。而朋友认为应该使用_propertyName,因为google objc code style认为最好不要用self.propertyName

我没看过google objc code style,我只看过objective c programming guide。在我的理解里property的作用在于根据参数生成相应的getter和setter。self.propertyName本质上既是调用getter函数的,而_propertyName直接访问成员函数,因为相应参数生成的getter和setter是不会被调用的。

再说,我还是决定相信apple,而不是google,毕竟Objc还是apple在支持和维护。

上代码

重点来了,朋友为了说服我self.property是有问题的,发了一段代码过来,这段代码非常奇葩,可以点这里下载,或者直接看代码,代码不算很长,简单的说,是要实现一个功能,一个tableview右上角有一个刷新按钮,每次刷新会改变dataArray(setupData),然后刷新tableview。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
#import "ViewController.h"

@interface ViewController () <UITableViewDataSource, UITableViewDelegate>

@property (strong, nonatomic) UITableView *tableView;
@property (strong, nonatomic) NSArray *dataArray;
@property (assign, nonatomic) BOOL flag;

@end

@implementation ViewController

- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
{
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self)
    {
        //按这个按钮本来是tableview会变化的,但是现在调用了reloadData之后,不会调用cellForRowAtIndexPath这个方法。
        UIBarButtonItem *rightItem = [[UIBarButtonItem alloc] initWithBarButtonSystemItem:UIBarButtonSystemItemEdit target:self action:@selector(add:)];
        self.navigationItem.rightBarButtonItem = rightItem;

        //设置数据源
        [self setupData];
    }
    return self;
}

- (void)viewDidLoad
{
    [super viewDidLoad];
    [self.view addSubview:self.tableView];
}

#pragma mark - getter & setter

- (UITableView *)tableView
{
    if (_tableView == nil)
    {
        _tableView = [[UITableView alloc] initWithFrame:self.view.bounds
                                                  style:UITableViewStyleGrouped];
        _tableView.autoresizingMask = UIViewAutoresizingFlexibleHeight;
        _tableView.delegate = self;
        _tableView.dataSource = self;
    }
    return _tableView;
}

#pragma mark - UITableView Delegate & Datasource

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    return self.dataArray.count;
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *identifier = @"settingcell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:identifier];
    if (cell == nil)
    {
        cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:identifier];
    }

    cell.textLabel.text = [self.dataArray objectAtIndex:[indexPath row]];

    return cell;
}

- (void)setupData
{
    if (self.flag)
        self.dataArray = [NSArray arrayWithObjects:@"1", @"2", @"3", @"fuck", nil];
    else
        self.dataArray = [NSArray arrayWithObjects:@"1", @"2", @"3", nil];
    [self.tableView reloadData];

}

- (void)add:(id)sender
{
    self.flag = !self.flag;
    [self setupData];
}

@end

问题

这段代码是无效的,按下按钮之后,setupData被调用了,已经log确定dataArray已经改变,tableviewdelegatedatasource都设置正确,确定numberOfRowsInSection被调用,奇葩的是cellForRowAtIndexPath没有调用,故而tableview没有改变。

奇葩的来了

朋友跟我说,你只要把[self.tableview reloadData]改成[_tableview reloadData],他就生效了。是的,他就生效了。你设一个断点在这个地方,然后把self.tableview_tableviewpo出来,发现他们的指针是一样的。朋友说写这个代码的那货折腾了一天,百思不得其解,最后得出结论self.propertyName就是坑爹。

生效的修改方法

朋友提供的:

  1. 前面说的讲把[self.tableview reloadData]改成[_tableview reloadData]
  2. tableview的getter函数的init里面的self.view.bounds改成CGReckMake(0,0, 320, 480)

朋友试图用这个两个方法来说明,self.property是坑爹的。

我在初步debug的时候,由于我是property的拥护者,property自动生成setter和getter函数,我是不支持重写getter函数的,所以我将getter函数删掉,把初始化代码移到viewdidload里面。然后代码就生效了。

但是即使代码生效了,还是没有找到问题的关键,仍然没办法解释为什么[self.tableview reloadData]改成[_tableview reloadData]就能运行了,因为po出来的指针是完全一样的,这不科学。

真正的问题所在

在各种Stackoverflow,google无果之后,我还是着手准备深入debug。

通过各种断点和gdb,最后打印函数调用栈才让我发现了真正的问题所在。

整个程序的执行顺序是这样的:

  1. initWithNibName(执行到[self setupData],没执行完) –>
  2. 第一次setupData(执行到[self.tableView reloadData],没执行完) –>
  3. 第一次执行tableview getter(到init,调用self.view,没执行完)->
  4. viewDidLoad(到addSubview:self.tableView, 没执行完) –>
  5. 第二次执行tableview getter(问题在这里!第一次执行的时候没有init玩,所以又会执行一次!)->
  6. 回到4.viewDidLoad,这是add的subview是第二次的init而先init完的tableview –>
  7. 回到3.第一次执行getter,(又alloc了一次tableView,这是self.property指向的是第一次init而后init完成的tableview))

所以,显示在界面上的tableview根本不是self.tableview指向的tableview,故而根本没法刷新(cellForRowAtIndexPath,是当需要显示的时候才会调用的)。

那为什么把[self.tableview reloadData]改成[_tableview reloadData]就能生效了呢?因为这样在initWithNibName的第一次调用setupData,就不会在reload的时候调用tableview getter,也就不会有后面一连串的连锁反应。之后顺利在viewdidload的时候只调用一次,完成init。

知道了问题的关键,还能有各种各样让他生效的方法,就不吐槽了。

正确的写法

这段奇葩代码带给我最大的感触就是,不好好写规范的代码,各种问题都会坑死你。我认为规范的写法应该是

  1. 不要重写getter和setter函数,使用property生成的getter和setter
  2. 不要在vc的init的函数里面初始化,尤其是初始化视图。而应该在viewdidload里面初始化,保证self.view已经生成。(非ARC环境下还需要注意memory warning导致的viewdidload多次加载而多次初始化所带来的内存泄露问题。最安全的做法是lazy instantiation)
  3. 应该使用自顶向下的程序设计方法,保证程序的顺序执行和层次关系。不应该出现如上程序的跳来跳去的调用。

后记

帮人debug还是有好处的,让我结识了这位bug兄。也让我更加深入的了解了cocoa的变量访问机制,debug的时候顺带还测试了KVO。

Edit

我又重新去看了property和getter,setter的资料,也看了苹果对property的解释。最后我修正关于不要重写getter和setter函数的观点,更正为可以重写getter和setter,目的可以为lazy instantiation, UI updating, consistency checking,等。但需要注意如上程序的连锁反应。代码的灵活性和安全性

关于@property,经过和大家的讨论也有了一个结论:

Why property?

Most importantly, it provides safety and subclassablility for instance variables. Also provides “value” for lazy instantiation, UI updating, consistency checking, etc.

Lancy

11.27.2012

iOS

Comments