代码重构-函数

阅读 331
收藏 20
2018-01-19
原文链接:www.jianshu.com

写在文前:大部分程序员都能写出计算机可以理解的代码,唯有优秀的程序员才能写出让人容易理解的代码

从某种程度上来说,一段有逻辑的函数是项目运行的最小单位(自己编的),所以如何写好函数至关重要。

以下简单讲述定义一个合理函数的规则

首要规则-短小且单一

短小很好理解,能短尽量短,经常读别人代码的人都清楚,如果一个函数一屏能看个大概,那读起来就很惬意。反之,看个函数,要滑好几屏,看到后面还得翻回去看看之前定义的变量是什么意思。这样的函数就显得有些失败。

个人建议函数的长度应该在一个到一个半屏幕之内,大致100行之内吧。

那单一又如何解释呢?文章开头说过,函数可以是项目运行的最小单位,那么既然是最小单位,所负责的任务应当独立且不能繁杂。

就比如登录模块
获取用户名密码,发送登录请求-->成功回调/失败回调
从单一原则来划分的话,一个登录操作应该分为上述两个函数。

有的开发者会习惯性的写在一个函数内,大多数情况下是没有问题的,在代码逻辑不复杂时,也不会造成函数行数过长的问题。但是这是如果在成功回调中有保持账号信息或者其他操作的话,还是得重开一个函数。

别重复

重复写着类似的代码是一个很大的问题,哪怕几个函数内只有部分代码是重复的,你也应该尽量的抽离出来。为了防止以后再修改相关代码时,需要改动多处,这种情况也往往是出现bug的情况。

保持一个抽象层级

抽象层级什么意思呢?类似于等级制度,或者说依赖关系。 举个最简单的列子,会员VIP,1J,2J,3J分别是三个层级,从依赖关系来看2J需要你先是1J。

从代码层面来说的话,比如创建视图这么一个行为。他的抽象层级可以简单的从高到低划分为。

创建视图->获取视图元素->拼接视图参数

private void createView(){
    getView();  
}

private void getView(){
    getViewParam();
}

缩减switch语句

想要写出简短的switch语句很难,因为switch本身就是为了处理复杂的逻辑而被创造出来的。

public void orderSalary(Person person){
    switch(person.type){
        case "doctor":
            float fixedSalary=123.0;
            float bonus=234.0;
            ....
            break;
        case "teacher":
            float fixedSalary=1235.0;
            float bonus=2344.0;
            ....
            break;
        ....
        default:
    }
}

就拿以上这个简单的薪资支付逻辑来看,如果薪资计算的方法简单,只需要几条公式就能表达清楚,我相信很多人可能就把相关的逻辑直接写在switch语句中。

然而这时,需求变动,加入了几个新的职业,比如厨师,助理等。
orderSalary可能就变成以下这个样子

public void orderSalary(Person person){
    switch(person.type){
        case "doctor":
            //TODO 计算医生薪水的相关逻辑
            break;
        case "teacher":
            //TODO 计算老师薪水的相关逻辑
            break;
        case "cook":
            //TODO 计算厨师薪水的相关逻辑
            break;
        case "assistant":
            //TODO 计算助理薪水的相关逻辑
            break;
        ....
        default:
    }
}

到了这个地步,就算薪水的逻辑在简单,orderSalary这个函数的长度也容易变得不那么好看。

既然问题出现了,那就来想想解决方式。 最简单的方式当然是将计算薪水的逻辑抽离出来,独立成一个函数。

public void orderSalary(Person person){
    switch(person.type){
        case "doctor":
            calculateDoctorSalary();
            break;
        case "teacher":
            calculateTeacherSalary();
            break;
        case "cook":
            calculateCookSalary();
            break;
        case "assistant":
            calculateAssistantSalary();
            break;
        ....
        default:
    }
}

private void calculateDoctorSalary(){}
private void calculateTeacherSalary(){}
private void calculateCookSalary(){}
private void calculateAssistantSalary(){}

就在你为解决一个问题洋洋得意的时候,老板又说把每个员工发工资的日期,方式等都记录下来!

这时你不得不在诸如calculateDoctorSalary()下在加上calculateDoctorSalaryDay()deliveryDoctorSalary()等函数。

写到这里转头一看,switch中的代码又变长了不少,同时你还要忧心忡忡的担忧,是否还会往这里加业务。

真到了这种时候,我推荐以工厂模式的方式来解决类似问题。

public abstract class Person{
    public abstract Float calculateDoctorSalary();
    public abstract int calculateDoctorSalaryDay();
    public abstract String deliveryDoctorSalary();
}   
.....

//函数
public void orderSalary(Person person){
    switch(person.type){
        case "doctor":
            return new Doctor(person);
        case "teacher":
            return new Teacher(person);
            break;
        ....
        default:
    }
}

控制参数数量

参数越多,意味个你这个函数越难以控制,同时也会增加测试的难度。 参数尽力控制在三个以内,如果超过三个,可以考虑用参数对象来代替。

拿一个简单列子来说明,

private void setUpTime(int year,int month,int day){
    //TODO 
}


private void setUpTime(DataInfo data){
    //TODO
}

不管是看函数长度,可读性还是测试难度来看,第二种写法都会第一种写法更合适。

避免帮倒忙

函数归根到底还是属于类中的元素,有的时候一个不恰当的操作可能会修改类中的变量,从而引起一些无法预期的变化。

private boolean chechUserValid(){
    String userId=getUserId();

    if(userId.isValid()){
        initUserInfo();
        return true;
    }
    return false;
}

这段代码咋看之下没有什么问题,userId可用时,初始化用户信息,并返回true。

这样的逻辑在正常流程是不会有问题的,验证用户id-->可用-->初始化数据。

当如果在别的模块也用到这个函数的时候就可能会有问题,比如支付的时候也要验证一下userId是否有效。这个时候协作者为了验证userId的有效性而调用时,就会出现问题。

你也可以修改函数名为 checkUserValidAndInitUserInfo()或者加上注释来提醒他人具体的用处。当然这样就违反了一个函数只做一件事,以及函数名过长的问题。

正确的做法就该是只是检验而已

抽离 try/Catch 语句

java中比较常见的错误拦截就是 try/catch语句了
使用 try/catch 的时候应该要注意几点

  1. try/catch 代码单独成函数
  2. try/catch 内的代码也尽量单独成函数

比如删除页面以及相关的引用逻辑 如下

private void AAA(){
    .....
    try{
        deletePage(page);
        deleteReference(page.name);
        deleteConfig(page.key);     
    }catch(){
        //TODO 错误处理
    }
    ....
}

但 try/catch 代码块上下都有复杂的逻辑的时候 AAA()这个函数阅读起来就十分不友好

改进

private void AAA(){
    .....
    delete(page);
    ....
}

private void delete(Page page){
    try{
        deletePage(page);
        deleteReference(page.name);
        deleteConfig(page.key);     
    }catch(){
        //TODO 错误处理
    }
}

这么一改,职责就清晰了许多,对于AAA()来说,delete(page)只是一个普通的函数引用,不需要多考虑其他。对于delete(page)自身来说,只需要关注删除逻辑和错误处理。

再改进

private void AAA(){
    .....
    delete(page);
    ....
}

private void delete(Page page){
    try{
        deletePageAndReference(page);   
    }catch(){
        //TODO 错误处理
    }
}

private void deletePageAndReference(Page page) throw Exception{
    deletePage(page);
    deleteReference(page.name);
    deleteConfig(page.key); 
}

这样的话,delete(page)这个函数就需要关注错误处理的问题,具体的业务下发到deletePageAndReference(page)中。

当然第二次的改进不是必须的,具体如何处理还是得看具体情况。

最后的建议

文章中讲述了这么多的原则,但你不要在一开始的时候就想着遵循全部的原则,因为这很困难。

首先确保功能逻辑的完整和正确,然后在一步步打磨代码,一步步的改进。

评论