4

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.

Gordon Gustafson
  • 40,133
  • 25
  • 115
  • 157
Arvanem
  • 1,043
  • 12
  • 22
  • Why are you assuming `foreach` is more efficient compared to `for` ? – nos Apr 25 '10 at 15:03
  • For-each is newer and I blindly assumed it would naturally be more efficient. Another assumption bites the dust, particularly thanks to CrazyJugglerDrummer's answer. – Arvanem Apr 25 '10 at 15:06
  • Taking this question at face value, of course it makes no sense to make a collection. However: the real question is why are you passing raw `int` values around anyway? Don't you have a `DicePool` made up of a collection of dice? Won't you at some point want to know their `lastThrow` result, etc. Or, to put it another way, where did you get the number of throws you need? Probably from some kind of collection that you could iterate over... – Dan Rosenstark Apr 25 '10 at 15:59
  • @yar Thanks for your comment. Surprisingly, I have no DicePool class. I get the number of throws I need directly from the value of the attributes and skills of the entity I wish to simulate the dice rolls for. The mechanics of Shadowrun do not need to know the lastThrow result, etc, but only the exact size of the dicePool. – Arvanem Apr 25 '10 at 21:03
  • I guess that works, unless you want to check if one of the dice is loaded :) – Dan Rosenstark Apr 25 '10 at 22:48

5 Answers5

12

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

Gordon Gustafson
  • 40,133
  • 25
  • 115
  • 157
4

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..

Jack
  • 131,802
  • 30
  • 241
  • 343
2

(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.

cletus
  • 616,129
  • 168
  • 910
  • 942
2

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.

  • Thank you Jacob for your answer. You can rest easy, there is not more than meets the eye here. The context is nothing more than simulating dice rolls for a boardgame RPG called Shadowrun. – Arvanem Apr 25 '10 at 15:12
1

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.

duffymo
  • 305,152
  • 44
  • 369
  • 561