CODEWITHSUNDEEP
javafor-loopforeachpremature-optimization
Arvanem
Arvanem

Reputation: 1053

Calling a method n times: should I use a converted for-each loop or a traditional for loop?

Given the need to loop up to an arbitrary int value, is it better programming practice to convert the value into an array and for-each the array, or just use a traditional for loop?

FYI, I am calculating the number of 5 and 6 results ("hits") in multiple throws of 6-sided dice. My arbitrary int value is the dicePool which represents the number of multiple throws.

As I understand it, there are two options:

  1. Convert the dicePool into an array and for-each the array:

    public int calcHits(int dicePool) {
   int[] dp = new int[dicePool];
   for (Integer a : dp) {
     // call throwDice method
   }
}

  2. Use a traditional for loop:

    public int calcHits(int dicePool) {
   for (int i = 0; i < dicePool; i++) {
     // call throwDice method
   }
}

My view is that option 1 is clumsy code and involves unnecessary creation of an array, even though the for-each loop is more efficient than the traditional for loop in Option 2.

Upvotes: 4

Views: 834

Answers (5)

Gordon Gustafson
Gordon Gustafson

Reputation: 41209

At this point, speed isn't important (insert premature-optimization comment ;). What matters is how quickly you can understand what the code does, which is to call a method dicePool times.

The first method allocates an array of size dicePool and iterates through its values, which happens to run the loop body dicePool times (I'll pretend you meant int instead of Integer to avoid the unrelated autoboxing issue). This is potentially inefficient for the computer running the code, but more importantly it's inefficient for the human reading the code as it's conceptually distant from what you wanted to accomplish. Specifically, you force the reader to think about the new array you've just made, AND the value of the variable a, which will be 0 for every iteration of the loop, even though neither of those are related to your end goal.

Any Java programmer looking at the second method will realize that you're executing the loop body dicePool times with i 'counting up' to dicePool. While the latter part isn't especially important, the beginning is exactly what you meant to do. Using this common Java idiom minimizes the unrelated things a reader needs to think about, so it's the best choice.

When in doubt, go with simplicity. :D

Upvotes: 12

Jacob O&#39;Reilly
Jacob O&#39;Reilly

Reputation: 136

What makes you think that the for-each loop is more efficient?

Iterating over a set is very likely less efficient than a simple loop and counter.

It might help if you gave more context about the problem, specifically whether there's more to this question than choosing one syntax over the other. I am having trouble thinking of a problem to which #1 would be a better solution.

Upvotes: 2

cletus
cletus

Reputation: 625097

(2) is the obvious choice because there's no point in creating the array, based on your description. If there is, of course things change.

Upvotes: 2

Jack
Jack

Reputation: 133587

Why would you need to allocate an array to loop over a variable that can be safely incremented and used without any need of allocation?

It sounds unecessarily inefficient. You can need to allocate an array if you need to swap the order of ints but this is not the case. I would go for option 2 for sure.

The foreach is useful when you want to iterate on a collection but creating a collection just to iterate over it when you don't need it is just without sense..

Upvotes: 4

duffymo
duffymo

Reputation: 308813

I wouldn't write the first one. It's not necessary to use the latest syntax in every setting.

Your instinct is a good one: if it feels and looks clumsy, it probably is.

Go with #2 and sleep at night.

Upvotes: 1

Related Questions