Redundant Code Constructions

Question

The most blatantly redundant code construct I often see involves using a code sequence

if (condition) return true; else return false; 

instead of just writing

 return (condition); 

I saw this beginner error in all languages: from Pascal and C to PHP and Java. What other constructions do you note in the code review?

Answer 1

 if (foo == true) { do stuff } 

I keep telling the developer what it should be

 if ((foo == true) == true) { do stuff } 

but he has not yet received a hint.

Answer 2

 if (condition == true) { ... } 

instead

 if (condition) { ... } 

Edit:

or worse, a circumvention of the conditional test:

 if (condition == false) { ... } 

which is easy to read as

 if (condition) then ... 

Answer 3

Using comments instead of a control source:
Commenting out or renaming functions instead of deleting them and trusting that the original control can return them to you if necessary.
-Adding comments such as “Change RWF” rather than just making changes and letting the source control assign the blame.

Answer 4

Somewhere I noticed this thing, which I consider the top of Boolean redundancy:

 return (test == 1)? ((test == 0) ? 0 : 1) : ((test == 0) ? 0 : 1); 

:-)

Answer 5

The announcement is separate from the destination in languages ​​other than C:

 int foo; foo = GetFoo(); 

Answer 6

The backup code alone is not an error. But if you are really trying to save every character

 return (condition); 

too redundant. You can write:

 return condition; 

Answer 7

 void myfunction() { if(condition) { // Do some stuff if(othercond) { // Do more stuff } } } 

instead

 void myfunction() { if(!condition) return; // Do some stuff if(!othercond) return; // Do more stuff } 

Answer 8

I once had a guy who repeatedly did this:

 bool a; bool b; ... if (a == true) b = true; else b = false; 

Answer 9

The return is useless at the end:

  // stuff return; } 

Answer 10

Using .tostring for a string

Answer 11

Setting the exit statement as the first statement in a function to disable the execution of this function, instead of one of the following options:

• Remove function completely
• Commenting function body
• Saving function, but deleting all code

Using exit as the first statement makes it very difficult to determine, you can easily read it.

Answer 12

Fear of scratch (this can also lead to serious problems):

 if (name != null) person.Name = name; 

If reservation (not using another):

 if (!IsPostback) { // do something } if (IsPostback) { // do something else } 

Fallback checks (Split never returns null):

 string[] words = sentence.Split(' '); if (words != null) 

More about checks (the second check is redundant if you are going to do a loop)

 if (myArray != null && myArray.Length > 0) foreach (string s in myArray) 

And my favorite for ASP.NET: Scattered DataBind throughout the code to render the page.

Answer 13

Copy the backup:

 if (x > 0) { // a lot of code to calculate z y = x + z; } else { // a lot of code to calculate z y = x - z; } 

instead

 if (x > 0) y = x + CalcZ(x); else y = x - CalcZ(x); 

or even better (or more confusing)

 y = x + (x > 0 ? 1 : -1) * CalcZ(x) 

Answer 14

Allocating items on the heap instead of the stack.

 { char buff = malloc(1024); /* ... */ free(buff); } 

instead

 { char buff[1024]; /* ... */ } 

or

 { struct foo *x = (struct foo *)malloc(sizeof(struct foo)); x->a = ...; bar(x); free(x); } 

instead

 { struct foo x; xa = ...; bar(&x); } 

Answer 15

The most common redundant code construct that I see is code that is never called anywhere in a program.

Others are design patterns in which it makes no sense to use them. For example, write "new BobFactory (). CreateBob ()" everywhere, and not just write "new BobFactory ()."

Removing unused and unnecessary code can significantly improve the quality of the system and the team's ability to maintain it. The benefits often hit teams that have never considered removing unnecessary code from their system. Once I conducted a code review, sitting with a team and deleting more than half of the code in my project, without changing the functionality of my system. I thought they were offended, but they often asked me about design advice and feedback after that.

Answer 16

I often come across the following:

 function foo() { if ( something ) { return; } else { do_something(); } } 

But this does not help to tell them that it is useless here. It should be either

 function foo() { if ( something ) { return; } do_something(); } 

or - depending on the length of the checks that run before do_something ():

 function foo() { if ( !something ) { do_something(); } } 

Answer 17

From the nightmare code reviews .....

 char s[100]; 

followed by

 memset(s,0,100); 

followed by

 s[strlen(s)] = 0; 

with lots of nasty

 if (strcmp(s, "1") == 0) 

clogged up about code.

Answer 18

Using an array when you want to set the behavior. You need to check everything to make sure it is not in the array before inserting it, which makes your code longer and slower.

Answer 19

Standby Calls .ToString ():

 const int foo = 5; Console.WriteLine("Number of Items: " + foo.ToString()); 

Unnecessary string formatting:

 const int foo = 5; Console.WriteLine("Number of Items: {0}", foo);