Which block of code is 'better'?

Which block of code is 'better'?

In order to promote good programming habits and increase the efficiency of my code (Read: "My brother and I are arguing over some code"), I propose this question to experienced programmers:

Which block of code is "better"? For those who can't be bothered to read the code, is it worth putting a conditional within a for-loop to decrease the amount of redundant code than to put it outside and make 2 for-loops? Both pieces of code work, the question is efficiency vs. readability.

    - (NSInteger)eliminateGroup {             NSMutableArray *blocksToKill = [[NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity] retain];             NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];             Block *temp;             NSInteger chargeTotal = 0;  //Start paying attention here              if (numOfBlocks > 3)              	for (NSUInteger i = 0; i < [blocksToKill count]; i++) {             		temp = (Block *)[blocksToKill objectAtIndex:i];             		chargeTotal += temp.charge;             		[temp eliminate];             		temp.beenCounted = NO;             	}             }             else {             	for (NSUInteger i = 0; i < [blocksToKill count]; i++) {             		temp = (Block *)[blocksToKill objectAtIndex:i];             		temp.beenCounted = NO;             	}             }	             [blocksToKill release];             return chargeTotal;         } 


        - (NSInteger)eliminateGroup {             NSMutableArray *blocksToKill = [[NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity] retain];             NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];             Block *temp;             NSInteger chargeTotal = 0;  //Start paying attention here              for (NSUInteger i = 0; i < [blocksToKill count]; i++) {                 temp = (Block *)[blocksToKill objectAtIndex:i];                 if (numOfBlocks > 3) {                     chargeTotal += temp.charge;                     [temp eliminate];                 }                 temp.beenCounted = NO;             }             [blocksToKill release];             return chargeTotal;         } 

Keep in mind that this is for a game. The method is called anytime the user double-taps the screen and the for loop normally runs anywhere between 1 and 15 iterations, 64 at maximum. I understand that it really doesn't matter that much, this is mainly for helping me understand exactly how costly conditional statements are. (Read: I just want to know if I'm right.)

[iPhone SDK] [Newbie] UITextView textViewDidChange will not be called?


conflict between JQuery slider and iui (iphone)
There is no way to answer this without defining your requirements for "better". Anyone know how to slide in a UIDatePicker like keyboard?Is it runtime efficiency? compiled size? code readability? code maintainability? code portability? code reuseability? algorithmic provability? developer efficiency? (Please leave comments on any popular measurements I've missed.). Managing data for UITableViews Sometimes complete runtime efficiency is all this matters, although not as often as people generally imagine, as you commit a nod towards in your question—although this is at least easy to test! Often it's a mix of all these concerns, and you'll have to make a subjective judgement in the end.. How to get a C method to accept UIImage parameter? Every answer here is applying a personal mix of these aspects, and people often receive into vigorous Holy Wars for the reason this everyone's right—in the right circumstance. how to release uiview added as subview in scrollview?These approaches are ultimately wrong. Populate a UITableView with dataThe only correct approach is to define what matters to you, and then measure against it.. Dynamically change url or WordPress theme if UserAgent is iPhone


The first code block is cleaner and more efficient for the reason this the check numOfBlocks > 3 is either true or false throughout the whole iteration.. The second code block avoids code duplication and might therefore pose lesser risk. However, it is conceptually more complicated.. The second block must be improved by adding.
bool increaseChargeTotal = (numOfBlocks > 3) 
before the loop and then using this boolean variable instead of the actual check inside the loop, emphasizing the fact this during the iteration it doesn't change.. Personally, in this case I would vote for the first option (duplicated loops) for the reason this the loop bodies are small and this shows clearly this the condition is external to the loop; also, it's more efficient and might fit the pattern "make the common case fast"..


All another things being equal, having two separate loops will generally be faster, for the reason this you did the test once instead of every iteration of the loop. The branch inside the loop each iteration will often slow you down significantly due to pipeline stalls and branch mispredictions; however, since the branch always goes the same way, the CPU will almost certainly predict the branch correctly for every iteration except for the first few, assuming you're using a CPU with branch prediction (I'm not sure if the ARM chip used in the iPhone has a branch predictor unit).. However, ananother thing to consider is code size: the two loops approach generates a lot more code, especially if the rest of the body of the loop is large. Not only does this increase the size of your program's object code, although it also hurts your instruction cache performance -- you'll receive a lot more cache misses.. All things considered, unless the code is a significant bottleneck in your application, I would go with the branch inside of the loop, as it leads to clearer code, and it doesn't violate the don't repeat yourself principle. If you make a change to once of the loops and forreceive to change the another loop in the two-loops version, you're in for a world of hurt..


I would go with the second option. If all of the logic in the loop was completely different, then it would make sense to make 2 for loops, although the case is this any of the logic is the same, and any is additional based upon the conditional. So the second option is cleaner.. The first option would be faster, although marginally so, and I would only use it if I found there to be a bottleneck there..


You would probably waste more time in the pointless and unnessesary [blocksToKill retain]/[blocksToKill release] at the start/end of the method than the time taken to execute a few dozens comparisons. There is no need to retain the array since you wont need it after you return and it will never be cleaned up before then.. IMHO, code duplication is a leading cause of bugs which should be avoided whenever possible.. Adding Jens recomendation to use fast enumeration and Antti's recomendation to use a clearly named boolean, you'd receive any thing like:.
    - (NSInteger)eliminateGroup {         NSMutableArray *blocksToKill = [NSMutableArray arrayWithCapacity:rowCapacity*rowCapacity];         NSInteger numOfBlocks = (NSInteger)[self countChargeOfGroup:blocksToKill];         NSInteger chargeTotal = 0;          BOOL calculateAndEliminateBlocks = (numOfBlocks > 3);         for (Block* block in blocksToKill) {             if (calculateAndEliminateBlocks) {                 chargeTotal += block.charge;                 [block eliminate];             }             block.beenCounted = NO;         }         return chargeTotal;     } 
If you finish your project and if your program is not running fast enough (two big ifs), then you must profile it and find the hotspots and then you must determine whether the few microseconds you spend contemplating this branch is worth thinking around — certainly it is not worth thinking around at all now, which means this the only consideration is which is more readable/maintainable..


My vote is strongly in favor of the second block.. The second block makes clear what the difference in logic is, and shares the same looping structure. It is both more readable and more maintainable.. The first block is an case of premature optimization.. As for using a bool to "save" all those LTE comparisons--in this case, I don't think it will help, the machine language will likely require exactly the same number and size of instructions..


The overhead of the "if" test is a handful of CPU instructions; way less than a microsecond. Unless you think the loop is going to run hundreds of thousands of times in response to user input, that's just lost in the noise. So I would go with the second quick fix for the reason this the code is both smaller and easier to understand.. In either case, though, I would change the loop to be. for (temp in blocksToKill) { ... }. This is both cleaner-reading and considerably faster than manually receive ting each element of the array..


Readability (and thus maintainability) must and should be sacrificed in the name of performance, although when, and only when, it's been determined this performance is an issue. . The second block is more readable, and unless/until speed is an issue, it is better (in my opinion). During testing for your app, if you find out this loop is responsible for unacceptable performance, then by all means, seek to make it faster, even if it becomes harder to maintain. But don't did it until you have to..

96 out of 100 based on 71 user ratings 306 reviews