iOS

iOS代码中的不良实践与优化

作者：StephenZhu

不良实践的根源

  • 项目时间紧，没有时间去思考更好的写法
  • 认为花精力去优化代码结构不值得

第一种原因，一般是因为经验不足，通过学习，任何人都可以很本能的写出结构良好的代码。至于时间确实很紧，后面总会重构的时间。

第二种原因，我觉得要分项目看。如果只是第一个试水版本，没有问题。但是作为一个长久项目，千万不能抱有这种观点。不仅害己，更是害人。

最近做了一个项目，由于种种原因，我们的代码有很多问题。因为正在做重构，所以就把一些不良的实践总结出来。希望对大家有所帮助。

不良实践的根源

  • 项目时间紧，没有时间去思考更好的写法
  • 认为花精力去优化代码结构不值得

第一种原因，一般是因为经验不足，通过学习，任何人都可以很本能的写出结构良好的代码。至于时间确实很紧，后面总会重构的时间。

第二种原因，我觉得要分项目看。如果只是第一个试水版本，没有问题。但是作为一个长久项目，千万不能抱有这种观点。不仅害己，更是害人。

最近做了一个项目，由于种种原因，我们的代码有很多问题。因为正在做重构，所以就把一些不良的实践总结出来。希望对大家有所帮助。

不良实践1：控制变量的滥用

控制变量，一般以XXflag的形式出现在代码中，其作用就是处理各种变化的条件。产品经理写需求往往是从人的角度说的，会对某种行为加上很多条件。这种复杂的限制转换到代码中就变成了一堆的控制变量。当你的代码中充斥着许多零散的flag，就好像打满补丁衣服，让人看着很难受。比如下面的代码：

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
- (void)viewWillAppear:(BOOL)animated
{
    [super viewWillAppear:YES];
    [[AppDelegate appDelegate]hideTabbar:YES];

    if (refreshFlag == NO)
    {
        refreshFlag = YES;
    }
    else
    {
        self.currentPage = 0;
        [_data removeAllObjects];
        [self getCarsList];
    }
}

- (void)viewWillDisappear:(BOOL)animated
{
    [super viewWillDisappear:YES];
    
    [[AppDelegate appDelegate]hideTabbar:NO];
    refreshFlag = NO;
}

- (void)viewDidDisappear:(BOOL)animated
{
    refreshFlag = YES;
}

上面的代码取自于项目中的某个类，这里面已经出现了n次reflashFlag，而且这个类的其他地方还有出现。顾名思义，这是为了控制这个页面是否要从后台重新获取数据。但是在实际测试中，我发现了一些问题，想改却无从下手。这就好像地心说的百衲衣，每次发现新的星体，就要从头开始把所有的轨道都算一遍，累都累死了。而日心说通过简单的公式就能覆盖所有的星体。可见，正确的解决方式都是简单的，搞得太复杂，肯定从一开始就错了。

回到当前这个问题，为什么要加个刷新呢？那得从需求说起，当前页面是一个我的汽车列表页面，可能会有多辆车。点击其中某辆车就可以进行编辑。编辑完成之后回到上一个页面（我的汽车列表页面）就需要刷新。最开始实现的时候，在viewWillAppear里面每次都调一下后台请求。但是后来加了滑动返回，当从编辑页面往回退时，其实用户还没真正返回，后台请求就发出了。于是乎，就加了refreshFlag来进行控制。

仔细思考这个问题，刷新的真正原因和实现的代码完全是不对等的。是用户更新了汽车信息才需要刷新，而不是返回需要刷新。这时，我们又遇到了新的问题：类和类之间如何通信。因为刷新是在类A中发生的，而更新内容是在类B中发生的。这种情况下，我觉得用通知是最佳实践。如果内容更新，类B就抛出一个通知。A的接收函数更新标志位（是的，还是会出现flag，但只会出现两次，很容易控制）。然后再viewWillAppear的时候根据标志位进行刷新。这样逻辑就比较顺，bug就相对少一些。

PS：类与类之间还有加引用、Delegate、KVO等通信方式，为什么不用？

不良实践2：同一个类中代码重复出现

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
- (void)getCode
{
    [self hideKeyBord];
    if (tfTelNumber.text == nil || [tfTelNumber.text isEqualToString:@""]) {
        showErroMsg(@"请输入手机号");
        return;
    }
    else if(![CommonMethod validateMobile:tfTelNumber.text]){
        showErroMsg(@"手机号码不正确");
        return;
    }
    
    MBProgressHUD* hudProgress;
    __block int result;
    
    if (viewFlag == 1)
    {
        hudProgress = [[MBProgressHUD alloc] initWithWindow:[UIApplication sharedApplication].keyWindow];
        [self.view addSubview:hudProgress];
        [self.view bringSubviewToFront:hudProgress];
        hudProgress.labelText = @"正在获取验证码...";
        
        APIPackageCheckCodeGoHeader *goHead = [[APIPackageCheckCodeGoHeader alloc]init];
        APIPackageCheckCodeBackHeader *backHead = [[APIPackageCheckCodeBackHeader alloc]init];
        goHead.phone = tfTelNumber.text;
        goHead.type = @"2";

        [goHead makeDictionary];
        NSMutableDictionary *dicBack = [[NSMutableDictionary alloc]init];
        [hudProgress showAnimated:YES whileExecutingBlock:^{
            //        调用接口方法
            result = [GMPostServer GetServerBack:SERVER_CHECKCODE path_Param:nil query_Param:goHead.dicGo body_Param:nil method:GM_NETWORK_METHOD_GET returnValue:dicBack];
            [backHead getBodyDataItems:dicBack];
        
            
        }completionBlock:^{
            if (result == GM_NETWORK_NULL_BACK)
            {
                _timeCount = [kTimeCount intValue];
                self.btGetCode.enabled = NO;
                [self.btGetCode setTitle:[NSString stringWithFormat:@"%d秒后重发",_timeCount] forState:UIControlStateDisabled];
                self.timerGetCode = [NSTimer scheduledTimerWithTimeInterval:1 target:self selector:@selector(countDown) userInfo:nil repeats:YES];
            }
            else
            {
                showErroMsg(backHead.errorMsg);
            }
        }];
        
    }
    else
    {
        hudProgress = [[MBProgressHUD alloc] initWithWindow:[UIApplication sharedApplication].keyWindow];
        [self.view addSubview:hudProgress];
        [self.view bringSubviewToFront:hudProgress];
        hudProgress.labelText = @"正在获取验证码...";
        
        APIPackageCheckCodeGoHeader *goHead = [[APIPackageCheckCodeGoHeader alloc]init];
        APIPackageCheckCodeBackHeader *backHead = [[APIPackageCheckCodeBackHeader alloc]init];
        goHead.phone = tfTelNumber.text;
        goHead.type = @"1";
        
        [goHead makeDictionary];
        NSMutableDictionary *dicBack = [[NSMutableDictionary alloc]init];
        [hudProgress showAnimated:YES whileExecutingBlock:^{
            //        调用接口方法
            result = [GMPostServer GetServerBack:SERVER_CHECKCODE path_Param:nil query_Param:goHead.dicGo body_Param:nil method:GM_NETWORK_METHOD_GET returnValue:dicBack];
            [backHead getBodyDataItems:dicBack];
            
            
        }completionBlock:^{
            if (result == GM_NETWORK_NULL_BACK)
            {
                _timeCount = [kTimeCount intValue];
                self.btGetCode.enabled = NO;
                [self.btGetCode setTitle:[NSString stringWithFormat:@"%d秒后重发",_timeCount] forState:UIControlStateDisabled];
                self.timerGetCode = [NSTimer scheduledTimerWithTimeInterval:1 target:self selector:@selector(countDown) userInfo:nil repeats:YES];
            }
            else
            {
                showErroMsg(backHead.errorMsg);
            }
        }];
        
    }
}

上面这个函数就是发送一个请求，请求根据状态不同分成两种情况。这里有两个问题：

  • 函数较长，100行
  • 代码重复，if/else里面的代码基本一样

这里的解决方法显而易见，给函数加一个参数，然后请求的参数

1
goHead.type = @"1";

根据函数参数来确定，请求过程只需要写一边即可。这里的重复代码去起来比较简单，有些地方会有一些牵扯（尤其是前文提到的控制变量，这是并发症），但都是可以解决的，最多分几步解决（先解决控制变量，在解决重复代码）。

不良实践3：不同类中重复代码

1
2
3
4
5
6
7
8
9
10
11
12
if ([LCStringUtil isBlankString:self.changedphoneNumfield.text]||[LCStringUtil isBlankString:self.orignphoneNumfield.text]) {
     showErroMsg(@"号码不能为空!");
     return;
 }
 if ([self.changedphoneNumfield.text isEqualToString:self.orignphoneNumfield.text]) {
     showErroMsg(@"号码不能相同");
     return;
 }
 if (![CommonMethod validateMobile:self.changedphoneNumfield.text]) {
     showErroMsg(@"请输入正确的新手机号");
     return;
 }

以上这段代码是对手机号码做合理性验证的，在好几个类里面反复出现。除了代码重复，更大的问题是不一致性。这个页面做了两个条件的判断，那个页面做了三个条件的判断，提示语又有可能不一样，这些全都是bug。

处理这个问题，需要有一个验证类，处理各种验证：验证电话号码、密码……然后创建验证对象，调用一下就可以了。

1
2
TextValidator*  validator =  [TextValidator new];
[validator isValidPhoneNumber];

不同类中的重复代码有些也可以放在基类中，如下面的代码，定制导航栏的title，在此不作详细解释。

1
2
3
4
5
6
7
8
9
10
11
12
13
//添加标题
- (void)addTitle:(NSString*)titleStr{
    
    UILabel *lblbartitle = [[UILabel alloc] initWithFrame:CGRectMake(0, 0, 100, 44)];
    lblbartitle.text = NSLocalizedString(titleStr, nil);
    lblbartitle.textColor = LCRGBColor(61, 66, 69);
    lblbartitle.center = CGPointMake(SCREEN_WIDTH/2, 44);
    lblbartitle.textAlignment = NSTextAlignmentCenter;
    lblbartitle.font = [HSGBFONT systemFontOfSize:20];
    lblbartitle.backgroundColor = [UIColor clearColor];
    self.navigationItem.titleView = lblbartitle;
    
}

不良实践4：劳模函数

一个人如果是多面手，那值得表扬；一个函数做多件事情，就有问题。看看下面的代码：

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
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
- (void)requestCarList
{
    [arrayCar removeAllObjects];
    
    APIPackageGetCarListGoHeader *goQueryHeader = [[APIPackageGetCarListGoHeader alloc] init];
    goQueryHeader.page = @"0";
    goQueryHeader.size = @"100";
    
    [goQueryHeader makeDictionary];
    
    __block NSInteger result = 0;
    APIPackageGetCarListBackHeader *backHeader = [[APIPackageGetCarListBackHeader alloc] init];
    NSMutableDictionary *dicBack = [[NSMutableDictionary alloc] init];
    MBProgressHUD *hud = [[MBProgressHUD alloc] initWithWindow:[AppDelegate appDelegate].window];
    hud.labelText = @"正在查询车辆列表，请稍候...";
    [[[AppDelegate appDelegate] window] addSubview:hud];
    [hud showAnimated:YES whileExecutingBlock:^{
        result = [GMPostServer GetServerBack:SERVER_GETCARLIST path_Param:nil query_Param:goQueryHeader.dicGo body_Param:nil method:GM_NETWORK_METHOD_GET returnValue:dicBack];
    }completionBlock:^{
        [hud removeFromSuperview];
        [backHeader getBodyDataItems:dicBack];
        if (result == GM_POSTBACK_SUCCESS)
        {
            [arrayCar addObjectsFromArray:backHeader.pageInfo.content];
            
            pageController.alpha = arrayCar.count>3?1:0;

            int intNum = [backHeader.pageInfo.content count]/3+([backHeader.pageInfo.content count]%3 == 0?0:1);
            scrollCar.contentSize = CGSizeMake((SCREEN_WIDTH+1)*intNum, 50);
            pageController.numberOfPages = intNum;
            
            //imageSelect.hidden = NO;
            
            [self removeAllSubViews:scrollCar];
            
            //画上部车辆按钮
            
            CGRect imageViewCarRect = CGRectMake(25, IS_INCH_4?12:10, 57, 22);
            CGRect labelCarRect = CGRectMake(0, IS_INCH_4?44:37, 106, 14);
            
            if (arrayCar.count >0) {
                for (int i=0;i<[arrayCar count];i++)
                {
                    APIPackageGetCarListBackHeader_content *item = (APIPackageGetCarListBackHeader_content*)[arrayCar objectAtIndex:i];
                    
                    UIButton *btnCar = [UIButton buttonWithType:UIButtonTypeCustom];
                    btnCar.frame = CGRectMake(107*i, 0, 106, CarScorllHeight);
                    btnCar.backgroundColor = [UIColor clearColor];
                    [btnCar addTarget:self action:@selector(selectCar:) forControlEvents:UIControlEventTouchUpInside];
                    btnCar.tag = i;
                    [scrollCar addSubview:btnCar];
                    
                    UIImageView *imageViewCar = [[UIImageView alloc] initWithFrame:imageViewCarRect];
                    if (i == 0)
                    {
                        selectCar = 0;
                        [imageViewCar setImage:[UIImage imageNamed:@"carIcon"]];
                    }
                    else
                    {
                        [imageViewCar setImage:[UIImage imageNamed:@"car_touch_no.png"]];
                    }
                    [btnCar addSubview:imageViewCar];
                    imageViewCar.tag = i+100;
                    
                    UILabel *labelCar = [[UILabel alloc] init];
                    labelCar.frame = labelCarRect;
                    labelCar.backgroundColor = [UIColor clearColor];
                    if (i == 0)
                    {
                        labelCar.textColor = LCRGBColor(214, 62, 37);
                    }
                    else
                    {
                        labelCar.textColor = LCRGBColor(105, 105, 105);
                    }
                    
                    labelCar.textAlignment = NSTextAlignmentCenter;
                    labelCar.font = [HSGBFONT systemFontOfSize:14];
                    labelCar.text = [NSString stringWithFormat:@"%@ %@",[LCStringUtil isBlankString:item.carBrand]?@"":item.carBrand,[LCStringUtil isBlankString:item.carSeries]?@"":item.carSeries];
                    labelCar.tag = i+200;
                    [btnCar addSubview:labelCar];
                    
                    UIImageView *imageLine = [[UIImageView alloc] initWithFrame:CGRectMake(106+107*i, 0, 1 / [UIScreen mainScreen].scale, CarScorllHeight)];
                    imageLine.backgroundColor = LCRGBColor(211, 211, 211);
                    [scrollCar addSubview:imageLine];
                }
            } else {
                UIButton *btnCar = [UIButton buttonWithType:UIButtonTypeCustom];
                btnCar.frame = CGRectMake(0, 0, 106, CarScorllHeight);
                btnCar.backgroundColor = [UIColor clearColor];
                [btnCar addTarget:self action:@selector(selectCar:) forControlEvents:UIControlEventTouchUpInside];
                btnCar.tag = -1;
                [scrollCar addSubview:btnCar];
                
                UIImageView *imageViewCar = [[UIImageView alloc] initWithFrame:imageViewCarRect];
                [imageViewCar setImage:[UIImage imageNamed:@"jiajiacheliang"]];
                [btnCar addSubview:imageViewCar];
                
                UILabel *labelCar = [[UILabel alloc] init];
                labelCar.frame = labelCarRect;
                labelCar.backgroundColor = [UIColor clearColor];
                labelCar.textColor = LCRGBColor(105, 105, 105);
                labelCar.textAlignment = NSTextAlignmentCenter;
                labelCar.font = [HSGBFONT systemFontOfSize:14];
                labelCar.text = @"添加车辆";
                [btnCar addSubview:labelCar];
                
                UIImageView *imageLine = [[UIImageView alloc] initWithFrame:CGRectMake(106, 0, 1 / [UIScreen mainScreen].scale, CarScorllHeight)];
                imageLine.backgroundColor = LCRGBColor(211, 211, 211);
                [scrollCar addSubview:imageLine];
            }
        }
        else
        {
            [self.navigationController popViewControllerAnimated:YES];
            showErroMsg(backHeader.errorMsg);
        }
    }];
}

看函数名就是请求汽车列表，但是实现代码除了请求，还有一大块的创建UI。这部分代码完全可以再创建一个函数来实现。
任何时候，设计良好的函数只做一件事情。

不良实践5：劳模类

劳模类就是毫不相关的函数一起出现的地方，最常见的就是单例类，最明显的就是AppDelegate。

1
2
3
4
5
- (void)autologin :(NSDictionary *)launchOptions
- (BOOL)intervalSinceNowThanOneHour: (NSString *) theDate
- (void)requestUserInfo
- (void)showFaildMsg:(NSString*)msg
.......

上面的函数功能五花八门，因为图方便，放在AppDelegate中。开了不好的头，后面的开发者就会源源不断往里面加函数。现在第一个版本就已经500+行，那么后面的扩张就可想而知了。
单例作为一种设计模式，有其存在的意义。但因为没有严格的限制，很多人把它当做了全局变量在用。在面向对象的世界中，全局变量都是老鼠屎般的存在，必须清除干净。
任何时候，设计良好的类只做一类事情。