“恶心代码”帖子中的一个回复

Question

曾在一个帖子中回复过最恶心的代码，摘抄如下：public void process(File file) {
    if (file != null) {
        if (file.exists()) {
            if (!file.isDirectory()) {
                if (file.canRead()) {
                    // do something;
                }
            }
        }
    }
}详见下面这个帖子 23 楼的回复：你见过最恶心的代码是什么样子的？
http://topic.csdn.net/u/20111020/16/e04cf593-4789-418b-8142-88c231002402.html本来准备睡觉的，刚才翻了一下这个帖子，得，没法睡了，得发帖子了！打 23 楼后，回复的同学竟然都异口同声地表明这代码没有问题！我一直告诫组员，代码中的 for、if 的嵌套不应该超过两层，超过的话得想办法重构。从那个帖子的回复来看，难道是我错了？抑或是我太敏感，还是这个代码真的没有问题？纯技术帖，谢绝“顶”、“Mark”之类无意义的回复，谢谢配合！

Answer 1

当初老师教的也是这么说的 for  if 循环嵌套不要超过2到3层 
有代码执行效率方面和阅读代码方便的考虑吧  
以后继续保持  

Answer 2

import com.google.common.base.Preconditions.checkNotNull;
public void process(File file){
    checkNotNull(file);
    if(file.isFile() && file.canRead(){
        ...
    }
}

Answer 3

checkNotNull很多开源项目的做法是Assert.notNull(file,"参数不能为空");很多项目中都有自己的Assert类

Answer 4

代码中的 for、if 的嵌套三四层应该是可以接受的吧
只是可以接受,尽量不要这样写就好了

Answer 5

//一般我喜欢这么做
public void process(File file) {
    if (file == null)return;
    if(!file.exists())return;
    if(file.isDirectory())return;
    if(!file.canRead())return;
    .....
    doSomething
    .....
}   

Answer 6

怎么重构啊？给个例子，莫非就是把几个if弄到一块，弄个&&吗？

Answer 7

com.google.common.base.Preconditions.checkNotNull
这个类原来是androidSDK里的啊，终于找到了，谢谢，又开眼界了

Answer 8

哈哈~如果只在最里面的一层写 处理代码，感觉还可以啊~
如果每层if 都有不同处理话~那就要恶心死了~ 12L的 方法挺有意思的，可以借鉴啊~

Answer 9

google code 上有单独的 google guava

Answer 10

其实嵌套几层这个我觉得没什么硬性规定 虽然我也不喜欢嵌套很多层(最近正在用while和break将原本的嵌套给展开) 可有些时候这样写是必要的 比如上面的代码在不满足条件的时候要进行一些处理 而满足条件还要做其他验证的情况下 就要这么做不过这段代码倒是没必要 因为咋看他都可以写到一个IF里去..

Answer 11

if (file != null && file.exists() && !file.isDirectory() && file.canRead())  

Answer 12

我也一般使用这样的方法，呵呵。当然了，我在这里贴的只是个示例，实际上每一层中可能有很多代码，每个 if 可能还有 else 语句块，如果加上这些的话，这代码就没有那么好看了。

Answer 13

我个人感觉if嵌套太多层了，12L的写法比较正常

Answer 14

在性能一样的情况下，当然选择可读性高的表现方式了！
如果我的话，选下面这种写法：public void process(File file) {
    if (file != null && file.exists() && !file.isDirectory() && file.canRead()){
       // do something;
    }
}

Answer 15

一般null和""放一个条件里。
其他都是
if(xxx)xxxx;
if(xxx)xxxx;
if(xxx)xxxx;
if(xxx)xxxx;
...

Answer 16

看了上面的说的方法，大概有两种，
1. if（）return；
2. if（。。）所有条件&&到一起。
针对上面这个问题，用1比2更好些，至少我不太喜欢看太长的语句。同样期待楼主给出更好的方法

Answer 17

这个帖子我也回了，我的方法是用并且。
楼主一般使用return，是不是可读性好？

Answer 18

三种代码写法的比较：
代码：public class Test6 {
public static void main(String[] args) {
test1();
test2();
test3();
} static void test1() {
int t = 10; if (t > 0)
if (t < 20)
if (t != 15)
System.out.println(100);
} static void test2() {
int t = 10; if (t <= 0)
return;
if (t >= 20)
return;
if (t == 15)
return; System.out.println(100);
} static void test3() {
int t = 10; if (t > 0 && t < 20 && t != 15)
System.out.println(100);
}
}三个函数对应的字节码：  static void test1();
    Code:
       0: bipush        10
       2: istore_0      
       3: iload_0       
       4: ifle          27
       7: iload_0       
       8: bipush        20
      10: if_icmpge     27
      13: iload_0       
      14: bipush        15
      16: if_icmpeq     27
      19: getstatic     #5                  // Field java/lang/System.out:Ljava/io/PrintStream;
      22: bipush        100
      24: invokevirtual #6                  // Method java/io/PrintStream.println:(I)V
      27: return          static void test2();
    Code:
       0: bipush        10
       2: istore_0      
       3: iload_0       
       4: ifgt          8
       7: return        
       8: iload_0       
       9: bipush        20
      11: if_icmplt     15
      14: return        
      15: iload_0       
      16: bipush        15
      18: if_icmpne     22
      21: return        
      22: getstatic     #5                  // Field java/lang/System.out:Ljava/io/PrintStream;
      25: bipush        100
      27: invokevirtual #6                  // Method java/io/PrintStream.println:(I)V
      30: return          static void test3();
    Code:
       0: bipush        10
       2: istore_0      
       3: iload_0       
       4: ifle          27
       7: iload_0       
       8: bipush        20
      10: if_icmpge     27
      13: iload_0       
      14: bipush        15
      16: if_icmpeq     27
      19: getstatic     #5                  // Field java/lang/System.out:Ljava/io/PrintStream;
      22: bipush        100
      24: invokevirtual #6                  // Method java/io/PrintStream.println:(I)V
      27: return        从代码风格上说，test1和test2的写法逻辑比较清晰。但test1的写法处理较复杂逻辑时会由于嵌套过多而出现失误。test3的写法的判断条件往往是由逻辑运算得到的计算式或者简化式，其写法相比test1可读性会差点，对于复杂的逻辑会出现较长的逻辑公式；但通过逻辑运算有时可以简化判断条件。从效率上说，test1与test3生成的字节码完全相同，从最长路径(invokevirtual #6)来分析：test2到达System.out.println函数时间损耗要比test1和test3多(跳转的效率要比顺序结构来得低)，因此在一些对系统效率要求比较严格的需求中test2的方案是不可取的。

Answer 19

有时候程序判断出结果, 并不就是要 return.
我的写法如下:void funtcion()
{
   do {
      if (expression1)
         break;
      if (expression2)
         break;
      if (expression2)
         break;
      // do something;
   } while(false);
}

Answer 20

我感觉用&&运算进行合并 应该可以吧

Answer 21

我感觉这点效率在java里，真就无所谓。

Answer 22

这样的写法，只能说明coder心思细腻，稳重，小心谨慎

Answer 23

就是你太敏感了。
我见过最恶心的是这样的:
String s1,s2,s3,s4,s5;
（所有的变量名都是这样，方名就是m1,m2,m3,m4）
然后下面外加3000行业务处理罗辑。
恶心吧。我足足搞了三天才帮这东西除完所有的BUG。外加改了一下功能，这完全草泥马。。

Answer 24

public void process(File file) {
    if (file == null) 
         return;
    if (!file.exists()) 
         return;
    if (file.isDirectory()) {
           return;
    if (file.canRead()) {
        // do something;
    }
}
最好写成这个样子

Answer 25

额，最多写过五重的for循环，当时是公司考试，没时间考虑算法，我们项目经理给的评价是“很清晰的死磕”。
LZ的情况，我大都都是 IF(XX==XX)return;....

Answer 26

呵呵，后期维护？？？
根据SOLID原则，不存在后期维护
软件的更新不是去修改已经通过测试的代码！
而是用新的组件替换旧的组件，而只有一个原因才会导致组件的更新，
那就是设计接口的更新

Answer 27

其实 很多后期维护都是别人来做的 o(∩_∩)o 哈哈 所以随便写 至于比较好的写法如下if(a)
 return;
if(b)
 return;
if(c)
 return;
if(d)
{
   do something...
}这个框架原来里面大部分是这种写法。

Answer 28

我觉得你主楼贴的代码逻辑比12楼31楼的方法清晰。
因为这段代码是层层递进关系，所以用if嵌套比一堆return或一个if清晰。

Answer 29

我见过的最恶心的代码是这这样的：
void func1()
{
    func2();
}void func2()
{
    func3();
}void func3()
{
    func4();
}继续N层......

Answer 30

呵呵，凑个热闹，贴个C#的Codepublic void process(string filePath) 
{
    if(string.IsNullOrEmpty(filePath))
    {
         return;
    }
    
    if(File.Exist(filePath))
    {
        try
        {
            File file = new File(filePath);
            //do something
        }
        catch
        {
            return;
        }
    }
}

Answer 31

错了，修改一下
public void process(string filePath) 
{
    if(string.IsNullOrEmpty(filePath))
    {
         return;
    }
    
    if(File.Exist(filePath))
    {
        try
        {
            //do something
        }
        catch
        {
            return;
        }
    }
}

Answer 32

Yup.这个示例 楼主说的“恶心”写法更清晰面向对象编程是很强调 可读性 和 可维护性。个人倾向于提炼方法的做法(放哪里你知道的)：public void process(File file) {
       if (isFileReadable(file)) {
          // do something;
       }
}
public boolean isFileReadable(File file){
     //这里怎么写维护的一般不会关心，除非有逻辑错误
}

Answer 33

无聊一下， 重构代码，太佩服我自己了static bool Check_file_readable(File *file)
{    if (file == null && !file.exists()) {
        return false;
    }
    if (file_isDirectory) {
        return false;
    }    return file.canRead();
}public void process(File *file) {
    
    if (check_file_readable(file)) {
        // do something;
    }

Answer 34

public void process(File file) {
  try{
     //do something
   }catch(Exception)
{}
}

Answer 35

我记得重构还是那本上说过，如果有多个if返回的结果都是一样的，那就把他们放在一起用&&或||运算符，因为多一个if，也就意味着多一种分支，多一种情况

Answer 36

感觉只有一个问题，
只处理了if
没有处理else至于，2层、3层，没有必要太教条

Answer 37

但是，我好像记得有人提过：一个函数里面最好只有一个return

Answer 38

所谓的程序要有一个入口，一个出口。其实不必拘泥于教条，多个return也是没什么问题的。