For example, I have a for loop, which element 0 has additional function to run compared with other elements, my question is, should the additional function be:
1.place inside for loop
for(int i=0;i<this.arr.length;i++){
this.arr[i].a();
if(i==0){
this.arr[i].b();
}
}
2.handle separately from for loop
this.arr[0].a();
this.arr[0].b();
for(int i=1;i<this.arr.length;i++){
this.arr[i].a();
}
which one should I use?
2 Answers 2
There is a huge difference in both code snippets: if this.arr.length
is 0 then option 1 works as designed, whereas option 2 fails in an attempt to perform operations on not existing elements.
Option 2 would be an alternative only if you add a preliminary check to verify if the loop condition is not false from the start. You can do that only if the expression is guaranteed not to have any side effects.
In view of the risks to inadvertently introduce some bugs during the manual optimization, I'd recommend to keep option 1.
Premature optimization is the root of all evil
- Donald Knuth
In addition, I think that option 1 better communicates the intents: the corrected option 2 has half of its statement for optimizing the processing of the first iteration, so that the future maintainer will have to think twice if ever he/shee has to change the loop's inner statements.
My take on this, you should use option 2 whenever possible, but with a minor tweak if that leads to the same result. I would remove the call to .a() for the first element and have the loop start at index 0.
if (this.arr.length > 0)
{
this.arr[0].b();
}
for(int i=0;i<this.arr.length;i++){
this.arr[i].a();
}
If that changes the behavior, it might be possible to move the call to .b() for element 0, so it's after the for loop.
First of all, this makes your code easier to read. You clearly execute something on the first element of the array when that statement is not in a for loop for that array, because all the information that makes that clear is on one line of code. The 0 index and the execution of the method are on the same line. If you'd use option 1, the information would be divided over two lines. The if statement would hold the information that you are looking to do something with the 0 index element and the execution of the method would be on a different line.
Also, the fact that you need an if statement in option 1, adds to the complexity of the code.
A secondary reason for this is performance. In your example, this is hardly a factor as you are only comparing an int variable with a constant, but I do want to include it here as the examples are easily extrapolated into much more complex scenario's, in which case performance could become a significant factor.
-
you could also push the first line in last if the process is independant for each item.Walfrat– Walfrat2018年08月28日 08:29:22 +00:00Commented Aug 28, 2018 at 8:29
-
@Walfrat: do you mean in my code or in the code within the question?Jonathan van de Veen– Jonathan van de Veen2018年08月28日 08:36:11 +00:00Commented Aug 28, 2018 at 8:36
-
This presupposed that a() and b() have no side effects. Also, add the empty array guard this.arr[0].b(), and finally, probably move the b() line after the loop to most closely match the original ([0].a() then [0].b(), because yours has b() before [0..length].a()Kristian H– Kristian H2018年08月28日 12:22:16 +00:00Commented Aug 28, 2018 at 12:22
-
@Kristian H: If you read carefully, you'll notice I did actually state this right after the code example. I do agree a check is needed in case of an empty array, so I included this in the code example. Thanks for that.Jonathan van de Veen– Jonathan van de Veen2018年08月29日 07:19:20 +00:00Commented Aug 29, 2018 at 7:19
arr[0].b()
is executed afterarr[0].a()
? Is it really important thatarr[0].b()
is executed beforearr[1].a()
andarr[i].a()
? If so in an OO approach I'd pass the index to a method on of the objects stored in the array and let them decide on their own.