我有时也会写成这样
boolean flag = getMboValue().isNull(); if (flag) { this.getMboValue("vendor").setReadOnly(false); } else { this.getMboValue("vendor").setReadOnly(true); }
原因是这样方便下断点调试,比如 flag是true为正常,false是异常情况需要调试。写成一行断点就不好下了。
我也不反对这种情况直接写成一行,只要命名合理逻辑清晰可读就好
我反对以下这样一行的写法:
foo.setDisable( ! (bar.isEnable() && this.getValue().isNull()) );
肯定、否定、并且、取反来回兜几个圈后我脑子就冒烟了,智商欠费。
首先,很显然的代码审查员的建议写法除了增加代码量之外,在可读性方面并没有太多的提升,甚至因为代码量的提升带来更多的心智负担和出错的概率。
例如因为某个奇怪的失误把代码写成了这样:
boolean flag = getMboValue().isNull(); if (flag) { this.getMboValue("vendor").setReadOnly(false); } else { this.getMboValue("vendor").setReadOnly(false); }
所以,原本一个可能出现问题的地方现在变成了四五个。
除此之外,flag的命名不够meaning,而我也倾向于移除这个变量,毕竟这个表达式并不长。但与上面不同的是,我个人会建议完全弃用 ! 运算符,原因是这个运算符实在是太不够明显了。所以我个人建议的写法是:
this.getMboValue("vendor").setReadOnly( getMboValue().isNotNull() );
增加一个isNotNull方法来使得语义更为清晰。
或者:
this.getMboValue("vendor").setReadOnly( getMboValue().hasValue() );
在无法增加方法的情况下,我也会建议用 == false来代替 ! ,因为在字面上这样会更难以被忽略:
this.getMboValue("vendor").setReadOnly( getMboValue().isNull() == false );
比较一下:
this.getMboValue("vendor").setReadOnly( !getMboValue().isNull() );
很显然上面的写法在提醒看的人,嘿,哥们儿注意点,我这里是反的。
当然,抛开代码审查员的问题,作为一个一无所知的程序员来重新审视这段代码,我会发现Mbo这个缩写其实会非常的令人费解,而且getMboValue这个方法实在是出现了太多次,另外为代码中一个有this,另一个没有this,让这段逻辑更加离奇。
最后,童鞋你写大括号的习惯我很欣赏,趁着你们的代码审查员还没有让你把所有的大括号都变成半展开之前,赶紧换工作投入大C#的怀抱吧……到时候同样的代码可能只需要写成这样就可以了:
mbo["vendor"].ReadOnly = mbo["vendor"] != null;
景甜:抱歉,是我选的他。