冗余代码构造
我经常看到的最严重的冗余代码构造涉及使用代码序列
if (condition) return true; else return false;
而不是简单地写
return (condition);
我已经在各种语言中看到了这个初学者错误:从Pascal和C到PHP和Java。我们还会在代码审查中标记其他哪些此类构造?
解决方案
最后无用地返回:
// stuff return; }
在字符串上使用.tostring
if (condition == true) { ... }
代替
if (condition) { ... }
编辑:
甚至更糟并扭转条件测试:
if (condition == false) { ... }
容易理解为
if (condition) then ...
if (foo == true) { do stuff }
我一直告诉开发人员,应该这样做
if ((foo == true) == true) { do stuff }
但是他还没有得到提示。
void myfunction() { if(condition) { // Do some stuff if(othercond) { // Do more stuff } } }
代替
void myfunction() { if(!condition) return; // Do some stuff if(!othercond) return; // Do more stuff }
我曾经有一个人反复这样做:
bool a; bool b; ... if (a == true) b = true; else b = false;
将" exit"语句作为函数中的第一条语句来禁用该函数的执行,而不是以下选项之一:
- 完全删除功能
- 注释功能主体
- 保留功能但删除所有代码
使用exit
作为第一条语句很难发现,我们可以轻松阅读它。
使用C以外的其他语言与赋值分开声明:
int foo; foo = GetFoo();
使用注释而不是源代码控制:
-注释掉或者重命名功能,而不是删除它们并相信源代码控制可以在需要时为我们找回它们。
-添加诸如" RWF更改"之类的注释,而不仅仅是进行更改并让源代码控制负责。
我在某个地方发现了这个东西,我发现它是布尔冗余的顶峰:
return (test == 1)? ((test == 0) ? 0 : 1) : ((test == 0) ? 0 : 1);
:-)
我经常遇到以下情况:
function foo() { if ( something ) { return; } else { do_something(); } }
但是,这无助于告诉他们其他东西在这里没有用。必须是
function foo() { if ( something ) { return; } do_something(); }
或者取决于在do_something()之前执行的检查的长度:
function foo() { if ( !something ) { do_something(); } }
要设置行为时使用数组。在插入之前,我们需要检查所有内容以确保其不在数组中,这会使代码更长,更慢。
冗余代码本身并不是错误。但是如果我们真的想保存每个角色
return (condition);
也是多余的。你可以写:
return condition;
害怕空值(这也会导致严重的问题):
if (name != null) person.Name = name;
如果是多余的(不使用其他):
if (!IsPostback) { // do something } if (IsPostback) { // do something else }
冗余检查(Split从不返回null):
string[] words = sentence.Split(' '); if (words != null)
有关检查的更多信息(如果要循环,第二个检查是多余的)
if (myArray != null && myArray.Length > 0) foreach (string s in myArray)
对于ASP.NET,我最喜欢的是:在代码中分散放置" DataBind"以使页面呈现。
复制粘贴冗余:
if (x > 0) { // a lot of code to calculate z y = x + z; } else { // a lot of code to calculate z y = x - z; }
代替
if (x > 0) y = x + CalcZ(x); else y = x - CalcZ(x);
甚至更好(或者更模糊)
y = x + (x > 0 ? 1 : -1) * CalcZ(x)
在堆而不是堆栈上分配元素。
{ char buff = malloc(1024); /* ... */ free(buff); }
代替
{ char buff[1024]; /* ... */ }
或者
{ struct foo *x = (struct foo *)malloc(sizeof(struct foo)); x->a = ...; bar(x); free(x); }
代替
{ struct foo x; x.a = ...; bar(&x); }
来自噩梦般的代码评论.....
char s[100];
其次是
memset(s,0,100);
其次是
s[strlen(s)] = 0;
有很多讨厌
if (strcmp(s, "1") == 0)
乱七八糟的代码。
冗余.ToString()调用:
const int foo = 5; Console.WriteLine("Number of Items: " + foo.ToString());
不必要的字符串格式:
const int foo = 5; Console.WriteLine("Number of Items: {0}", foo);
我看到的最常见的冗余代码构造是从未在程序中的任何位置调用的代码。
另一个是在没有用处的情况下使用的设计模式。例如,在各处编写" new BobFactory()。createBob()",而不仅仅是编写" new Bob()"。
删除未使用和不必要的代码可以极大地提高系统的质量以及团队的维护能力。对于从未考虑从系统中删除不必要的代码的团队来说,好处通常是惊人的。我曾经与团队一起执行代码审查,并删除了项目中一半以上的代码,而没有更改他们系统的功能。我以为他们会被冒犯,但之后他们经常要求我提供设计建议和反馈。